backend: (x86) lowering of func.func and func.ret to x86_func #3918
Conversation
qaco
added
transformations
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3918 +/- ##
==========================================
+ Coverage 91.29% 91.31% +0.01%
==========================================
Files 466 467 +1
Lines 58012 58108 +96
Branches 5566 5579 +13
==========================================
+ Hits 52962 53060 +98
+ Misses 3622 3618 -4
- Partials 1428 1430 +2 ☔ View full report in Codecov by Sentry. |
qaco
force-pushed
the
hugo/func-to-x86
branch
from
4ee7380 to
528680f
Compare
| first_block = op.body.blocks.first | ||
|
|
||
| if first_block is None: | ||
| raise ValueError("Cannot lower external functions (not implemented)") |
There was a problem hiding this comment.
Can you please change these to DiagnosticException, and add filecheck stubs to exercise them? Ideally we want 100% code coverage for lowerings like this
| mov_op = x86.RM_MovOp( | ||
| r1=get_reg_op.result, | ||
| r2=get_sp_op.result, | ||
| offset=8 * (i + 1), |
There was a problem hiding this comment.
We don't have a good way to represent x86 vs x86-64 (and probably we might never need this), but maybe make the offset set (8 bytes) a bit more pronounced in this pass? Maybe a global constant or similar?
| func.func @foo_int() -> (i32,i32) { | ||
| %a = "test.op"(): () -> i32 | ||
| func.return %a,%a: i32,i32 | ||
| } |
| class LowerReturnOp(RewritePattern): | ||
| @op_type_rewrite_pattern | ||
| def match_and_rewrite(self, op: func.ReturnOp, rewriter: PatternRewriter): | ||
| if len(op.arguments) == 0: |
There was a problem hiding this comment.
this should be if not op.arguments instead, getting the length is potentially O(n), whereas truthiness is constant time and cheap
No description provided.