Question on arithmetic overflow detection support in C2

[Krystal Mok]

Hi all,

Just FYI, I've posted an updated version of the patch here:
https://gist.github.com/db03ab15ef8b76246b84#file_checked_add_prototype.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> 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 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/20120627/edf4a294/attachment.html