[DO NOT MERGE] Implement mov and movptr pseudoinstructions #403

sayon · 2024-02-25T22:47:41Z

Code Review Checklist

Purpose

Ticket Number

Requirements

Have the requirements been met?
Have stakeholder(s) approved the change?

Implementation

Logic Errors and Bugs

Can you think of any use case in which the
code does not behave as intended?
Can you think of any inputs or external events
that could break the code?

Error Handling and Logging

Is error handling done the correct way?
Should any logging or debugging information
be added or removed?
Are error messages user-friendly?
Are there enough log events and are they
written in a way that allows for easy
debugging?

Maintainability

Is the code easy to read?
Is the code not repeated (DRY Principle)?
Is the code method/class not too long?

Dependencies

Were updates to documentation, configuration, or readme files made as required by this change?
Are there any potential impacts on other parts of the system or backward compatibility?

Security

Does the code introduce any security vulnerabilities?

Performance

Do you think this code change decreases
system performance?
Do you see any potential to improve the
performance of the code significantly?

Testing and Testability

Is the code testable?
Have automated tests been added, or have related ones been updated to cover the change?
- For changes to mutable state
Do tests reasonably cover the code change (unit/integration/system tests)?
- Line Coverage
- Region Coverage
- Branch Coverage
Are there some test cases, input or edge cases
that should be tested in addition?

Readability

Documentation

Is there sufficient documentation?
Is the ReadMe.md file up to date?

Best Practices

Follow Single Responsibility principle?
Are different errors handled correctly?
Are errors and warnings logged?
Magic values avoided?
No unnecessary comments?
Minimal nesting used?

Experts' Opinion

Do you think a specific expert, like a security
expert or a usability expert, should look over
the code before it can be accepted?
Will this code change impact different teams, and should they review the change as well?

When converting bytes to cell cache the result of the conversion and reuse it instead of recalculating.

Replace lower switch pass with stich loweing in DAG builder The patch turns off lowerswitch pass, and change Selection DAG Builder so that it creates a tree with ult rather than slt branching for a switch.

Propagate constants to stack address In case a fuction access to an external frame outside of the entry basic block, the DAG is constructed so that the address is computed in entry and copied to a register. SelectAddress therefore can't infer that a constant could be involved in the computation and use Reg + Imm addressing mode. The patch introduces a pass that replicates the logic of propagation constants to a memory accessing operation. If the inital address is computed by expression 1, and none of it's patrital results is used more than once, the pass attempts to find expression 2, so that expression 1 is equal to expression 2 + constant. Then it replaces a memory access by expression 1 to a memory access by expression 2 + constant.

* Add the MUSL build script * Install ninja * Install the Rust target for MUSL * Install musl-gcc * Fix the LLVM output path * Fix the LLVM unit test path and target * Re-enable unit tests building * Remove the unit tests CI step Co-authored-by: Alex Z <a.zarudnyy@matterlabs.dev>

* Test only opt-modes for nightly * Cleanup the build scripts Co-authored-by: Alex Z <a.zarudnyy@matterlabs.dev>

Selection DAG Builder used to build jump tables for minsize attributed function when switch cases are dense. SyncVM doesn't allow jump tables yet.

SyncVMCodegenPrepare pass now eliminates such calls.

Despite the command-line option, LLVM unconditioally enables lookup table building for switch. These tables are global constants and thus every record there is 4 times as bigger as a single instruction.

Expand memcpy using gep instead of pointer arithrmetic to access elements, and move the logic to SyncVM target instead of making local changes in LowerMemIntrinsics. The patch also corrects the data layout for SyncVM.

Update runtime library to return iN as(3)* when appropriate.

The compiler put all the globals into `.data` section regardless of the initializer. Linker is to map `.data` to the beggining of the stack.

* adding ptr.add and ptr.pack * adding corresponding intrinsics * GEP on AS 3 pointers will be converted to intrinsics before lowering Co-authored with Dmitry Borisenkov <dmitriy.borisenkov89@gmail.com>

SyncVM threats generic address space pointers as a special data type. Only VM can produce them, and there are 3 operations that allowed: * ptr.add pointer, i256 * ptr.sub pointer, i256 * ptr.pack pointer, i256 Any other operation on a pointer would implicitly convert it to i256. The patch supports fat pointers in backen without introducing a EVT for them: * Select load/store iN as(3)** as ptr.add * Identify via the def-chain that pointer value is to be spilled, and generate ptr.add instead of add for the spill * Identify that pointer is to be reloaded and use ptr.add for that * Identify that pointer is to be moved and use ptr.add

* Make a copy of each far call runtime function and make each copy to take i8 addrspace(3)* as the first argument. * Handle COPY from a function parameter to a register when the parameter is a generic pointer.