dialects: Add LLVM InlineAsm op #2203
Conversation
JosseVanDelm
added
enhancement
|
Can you please add a test for the roundtrip with MLIR? |
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #2203 +/- ##
==========================================
+ Coverage 89.44% 89.45% +0.01%
==========================================
Files 319 322 +3
Lines 38709 38770 +61
Branches 5736 5742 +6
==========================================
+ Hits 34623 34683 +60
- Misses 3282 3289 +7
+ Partials 804 798 -6 ☔ View full report in Codecov by Sentry. |
|
Added @superlopuh ! |
| "asm_dialect": IntegerAttr.from_int_and_width(asm_dialect, 64), | ||
| } | ||
| if has_side_effects: | ||
| props["has_side_effects"] = UnitAttr() | ||
| if is_align_stack: | ||
| props["is_align_stack"] = UnitAttr() | ||
|
|
There was a problem hiding this comment.
| "asm_dialect": IntegerAttr.from_int_and_width(asm_dialect, 64), | |
| } | |
| if has_side_effects: | |
| props["has_side_effects"] = UnitAttr() | |
| if is_align_stack: | |
| props["is_align_stack"] = UnitAttr() | |
| "asm_dialect": IntegerAttr.from_int_and_width(asm_dialect, 64), | |
| "has_side_effects": UnitAttr() if has_side_effects else None, | |
| "is_align_stack": UnitAttr() if is_align_stack else None, | |
| } |
There was a problem hiding this comment.
can then probably move the dict literal into the super init call
There was a problem hiding this comment.
Pyright throws an error if I do that, cause props is not allowed to have a None as value in the dictionary.
I've seen some other shorthands here, but just like the poster here I prefer the original code:
https://stackoverflow.com/questions/14263872/only-add-to-a-dict-if-a-condition-is-met
There was a problem hiding this comment.
Wait a second, properties can't be missing, can you please check that in MLIR these are properties and not attributes by printing the generic syntax?
@superlopuh pointed out that #2203 contains an erroneous implementation of a UnitAttr being implemented as properties. This is incorrect, since upstream prints the UnitAttrs as attributes and properties. It was a bit confusing because some other ops in the LLVM dialect have the same mistake right now. This happens under the radar because in testing, properties are printed as attributes. This PR fixes the inline_asm operation.
This PR adds the LLVM InlineAsm op, which allows to bake in inline assembly from within MLIR. There's an additional attribute to select the inline assembly dialect (not dialect in context of MLIR, choices are AT&T or Intel). I just implemented this as an `IntegerAttr`.
This PR attempts to add the LLVM InlineAsm op, which allows to bake in inline assembly from within MLIR.
Some things I noted while making this:
The docs mention this:
I'm assuming this means that this constraint is never actually checked/verified 😅 ?
There's an additional attribute to select the inline assembly dialect (not dialect in context of MLIR, choices are AT&T or Intel). I just implemented this as an
IntegerAttr, is that okay? It looked likeLinkage(prior to this PR) was/is implemented differently from upstream in a similar way?Currently I have only implemented round-trip tests.
Happy to add any other tests if required.
CC @jorendumoulin