s390x: Basic support for IaddIfcout - #3012
Conversation
cfallin
left a comment
There was a problem hiding this comment.
Thanks!
Just a request for a little more detail/description below to help future readers, but otherwise LGTM.
As a meta-level comment, I do think that a more self-contained instruction here -- maybe "add and trap on overflow" or something like that? -- would probably be better, as it wouldn't require special handling of flags. (The original design was pretty x86-centric, for sure.) But that's a future refactor, and this should work for now!
| // can simply rely on the condition code here. However, the encoding is | ||
| // different. | ||
| assert!(condcode == IntCC::UnsignedGreaterThan); | ||
| cond = Cond::from_mask(2 | 1); |
There was a problem hiding this comment.
Could we add a comment here noting what these constants are (which condition flags) and how the combination of the add instruction generated by IaddIfcout and the condcode here will properly trap?
Perhaps also a note in unsigned_add_overflow_condition() would be helpful in order to describe how the condition code is actually specially handled in the one place this is known to be used.
This adds enough support for the IaddIfcout opcode to make the code emitted by dynamic_addr work on s390x. Note: On s390x, the condition code mask that has to be used to implement unsigned_add_overflow_condition does not match any of the masks for the "normal" condition codes, so this design is not really a good match for s390x ...
|
Added comments.
Either that, or at least some new value like IntCC::Carry (analogous to the existing IntCC::Overflow). |
This adds enough support for the IaddIfcout opcode to make the
code emitted by dynamic_addr work on s390x.
Note: On s390x, the condition code mask that has to be used to
implement unsigned_add_overflow_condition does not match any of
the masks for the "normal" condition codes, so this design is
not really a good match for s390x ...