Review request for JDK-8058610

[Attila Szegedi]

On Oct 21, 2014, at 1:31 PM, Marcus Lagergren <marcus.lagergren at oracle.com> wrote:

> An example in this comment would have helped me understand this sooner. Maybe you can add one? It would be appreciated.
> +                // Can reorder, but can't move conversion into the operand as the operation depends on operands
> +                // exact types for its overflow guarantees.

Modified it to:

// Can reorder, but can't move conversion into the operand as the operation depends on operands
// exact types for its overflow guarantees. E.g. with {L}{%I}expr1 {L}* {L}{%I}expr2 we are not allowed
// to merge {L}{%I} into {%L}, as that can cause subsequent overflows; test for JDK-8058610 contains
// concrete cases where this could happen.

> Can these be private?

They can, but then javac will add synthetic package-private accessors for access from CodeGenerator$TypeBounds. Matter of style, FWIW. I'll "privatize" them.
 
> 
> +    static Type booleanToInt(final Type t) {
> +        return t == Type.BOOLEAN ? Type.INT : t;
> +    }
> +
> +    static Type objectToNumber(final Type t) {
> +        return t.isObject() ? Type.NUMBER : t;
> +    }
> 
> Wasn’t it possible by solving this by always using an indy lmul(jj)j that might throw an UnwarrantedOptimismException containing a double, or is it too expensive to introduce the 53 bit check logic there?

Yes, it is possible (I say as much in this comment: <https://bugs.openjdk.java.net/browse/JDK-8058610?focusedCommentId=13566721&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13566721>), but it'd require a more extensive modification of code generator that I'd rather not embark on now. Namely, CodeGenerator.OptimisticOperation has an elaborate criteria for deciding whether it should indeed be optimistic or not (see setting of the isOptimistic flag in its constructor), and it'd have to accommodate the special case of BinaryNodes for arithmetic operations then.

> Are we using lmul(jj)j elsewhere and not checking this?

No. CodeGenerator's loadMUL and loadASSIGN_MUL are the only cases, and they're covered. FWIW, the tests exercise ADD, SUB, and MUL operations, as this was not constrained to multiplication only. Division and modulo are unaffected as they were treated as special cases anyway, as even without overflowing their results can lead out of the set of integral numbers into floating point numbers.

> 
> Otherwise +1
> 
> Regards
> Marcus
> 
> 
> On 21 Oct 2014, at 14:46, Attila Szegedi <attila.szegedi at oracle.com> wrote:
> 
>> Please review JDK-8058610 at <http://cr.openjdk.java.net/~attila/8058610/webrev.00> for <https://bugs.openjdk.java.net/browse/JDK-8058610>
>> 
>> Thanks,
>>  Attila.
>