[codegen] reintroduce support for carry and borrow instructions in RI…#1005
[codegen] reintroduce support for carry and borrow instructions in RI…#1005bnjbvr merged 5 commits intobytecodealliance:masterfrom
Conversation
|
Thanks for looking into this! Our legalization groups have historically grown in a very ad-hoc way, and this is proof they don't quite make sense. We could split them into smaller ones, and chain them together to keep the things working almost the same way. It would be interesting to make a legalization group that's not specific to RISC but to platforms which don't have flags, for one thing. |
…SC ISAs Reintroduce support for iadd carry variants and isub borrow variants for RISC ISAs which had been removed in bytecodealliance#961 and bytecodealliance#962 because of the lack of a proper flags register in RISC architectures.
8b1bb15 to
167cff0
Compare
|
Added a new legalization group and rebased. IIUC, this should've passed, but this clearly doesn't: it throws @bnjbvr @sunfishcode what might be wrong? |
|
@bjorn3 sorry, the new recipes were added rather hastily, and I didn't put a lot of thought into them. The updated version should look much more accurate. That said, I'm still getting |
| vec![ | ||
| def!((a, c1) = iadd_cout(x, y)), | ||
| def!(c_int = bint(c1)), | ||
| def!(c = ifcmp(uimm32_0, c_int)), |
There was a problem hiding this comment.
This would turn overflow into the ne flag, instead of the carry flag.
bnjbvr
left a comment
There was a problem hiding this comment.
Looking great, thanks! Very small comments, I'm happy to address them for you in a subsequent small PR, if you don't want to deal with those now.
|
(Actually added those for you! If the build passes, I'll squash everything.) |
…SC ISAs
Reintroduce support for iadd carry variants and isub borrow variants for
RISC ISAs which had been removed in
#961 and
#962 because of the lack
of a proper flags register in RISC architectures.
/cc @wingo @caitp
@sunfishcode @bnjbvr there's a slight problem. x86 and RISC seem to be reusing both
narrowandexpandso I cannot choose between the two different versions of the instructions (eg: since both x86 and RISC use narrow to expandiadd.i64, one would need to choose betweeniadd_c*andiadd_ifc*). I believe creating a specialized legalization file forriscvinmetaand adding the legalization specifically should help.