Question on arithmetic overflow detection support in C2

[John Rose]

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 INT_CC ⇒ INT_CC_PAIR 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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/attachments/20120619/61a166ae/attachment.html