Question on arithmetic overflow detection support in C2 (original) (raw)

Vladimir Kozlov vladimir.kozlov at oracle.com
Thu Jun 28 07🔞13 PDT 2012

This looks good. Add next line only in cmpOp() operand to matching signed.

Do not rename, just use 'n' as argument:

+CheckedAddINode* CheckedAddINode::make(Compile* C, Node* cmpadd) {

Did you tried INT_REG_mask() instead of INT_RAX_REG_mask() in checkedAddI_sum_proj_mask() to avoid using bound register in checkedAddI_rReg()? And did you tried match(Set dst (CheckedAddI dst src))?

Thanks, Vladimir

Krystal Mok wrote:

Hi all,

Just FYI, I've posted an updated version of the patch here: https://gist.github.com/db03ab15ef8b76246b84#filecheckedaddprototype.ver3.patch It pretty much implements what John suggested in a previous email (quoted below). This version still suffers from a couple of problem mentioned before: 1. It's emitting a jmpConU instead of a jmpCon during instruction selection. Is there a way to force it use jmpCon here? 2. I had to use a fixed register for the sum projection of CheckedAddI, otherwise I couldn't find a way to specify this projection should use the same register as "dst". - Kris On Wed, Jun 20, 2012 at 3:12 AM, John Rose <john.r.rose at oracle.com_ _<mailto:john.r.rose at oracle.com>> wrote: On Jun 19, 2012, at 7:07 AM, Krystal Mok wrote:

In the DivMod example, Div and Mod nodes with the same inputs start out floating separately, and get fused into a single DivMod late in Optimize(). The DivMod node type is a MultiNode with 2 projections, one for the div and the other for the mod. There's special treatment of DivMod in Matcher, and custom logic to match the projections.

If I add a new node type AddIOverflow following the same model, I might get it like this: https://gist.github.com/bdd13666e4796b09f36e This node type would have 2 projections just like DivMod, one for the add, and the other for the overflow condition. (rename INTCC ⇒ INTCCPAIR for clarity) Then there are two questions: 1. I'd need another new node type, presumably called "CheckAddIOverflow" here, that derives from CmpINode and acts as if it produces condition codes. AddI(a, b) and CheckAddIOverflow(a, b) float separately, just like the DivMod example. They get fused up late in Optimize() into the AddIOverflow shown above. Does this sound right? Yes, that sounds right. The names don't feel right, yet. What's the best term for a version of Add which is not subject to overflow? That's what the user will be trying to build. Maybe a word like "Checked" or "Safe" is best, rather than focusing on overflow. Maybe: AddI(a, b) plus CmpAddI(a, b) ⇒ CheckedAddI(a, b) plus two projections yielding the first two values. You might be right about adding two new BoolNode states. That way you can think in terms of: if ((c = a + b) != 0) … turning into c=AddI(a,b); cc=CmpAddI(a,b); if(Bool::nz(cc)) … And overflow turns into the natural special case supported by the HW. 2. With AddIOverflow going into the Matcher, how should I write the match rule in the ad file, to match: If(Bool(Proj(AddIOverflow(a, b)).overflow, ne), labl) ? Would it look like: match(If cop (AddIOverflow dest src)) ? (You have probably already noticed it; the matcher.cpp comment "Convert (If (Bool (CmpX A B))) into (If (Bool) (CmpX A B))" is highly relevant to matching If nodes. See also uses of BinaryNode.) Following the DivMod pattern, the matcher (in its current state) will have to match the CheckedAddI as a stand-alone node. Subtrees of matches can only have one use; this is a key matcher invariant. But every DivMod or CheckedAddI will have two uses: Its two projections. So you'll have: match(CheckedAddI); and the pre-existing rule: If(cop cmp) The cop will be Bool::ov (or Bool::nz in the example above) and the cmp will be CheckedAddI.cmp. You might think about extending the matcher machine to allow rules which specify the disposition of both projections: match( (If cop (Proj#1 (CheckedAddI a b))); (Set c (Proj#0 (CheckedAddI a b)))); This would be a Big Project. Long term, I believe it is worth thinking about ways to model instructions that do more than one thing at a time. If the pair of overflow/non-overflow conditions are in BoolTest and in cmpOp, perhaps I wouldn't need to match the If node, and could just let jmpCon rule handle it as-is. That way I just have to match AddIOverflow itself in the ad file, like DivMod. — John

More information about the hotspot-compiler-dev mailing list