S RFR: Lambda: verification error in generated lambda classes

[Karen Kinnear]

Thanks David. I had to also, but this seemed the cleanest approach.

thanks for the review and suggestion,
Karen

On Aug 29, 2013, at 10:09 PM, David Holmes wrote:

> On 30/08/2013 4:02 AM, Keith McGuigan wrote:
>> I think there's an extra set of parenthesis that isn't needed, but other
>> than that, thanks, looks good!
> 
> Agreed on all three counts (extra, not needed, good) :)
> 
> I still had to write out the truth table to be certain :)
> 
> Thanks,
> David
> 
>> 
>> On Thu, Aug 29, 2013 at 1:29 PM, Karen Kinnear <Karen.Kinnear at oracle.com
>> <mailto:Karen.Kinnear at oracle.com>> wrote:
>> 
>>    Keith, David,
>> 
>>    Many thanks for the suggestions. I think Keith's proposal is much
>>    simpler to read. I did
>>    a name change as well to hopefully make this more obvious.
>> 
>>    webrev.02:
>>> 
>>>        >> webrev: http://cr.openjdk.java.net/~acorn/8023872.02/webrev/
>>>        >> http://bugs.sun.com/view_bug.do?bug_id=8023872
>> 
>>    I redid the manual testing, and the longer tests are in progress again.
>> 
>>    thanks,
>>    Karen
>> 
>>    On Aug 28, 2013, at 10:31 PM, Keith McGuigan wrote:
>> 
>>>    I think it's ok as it is, if you need to check it in quick, but I
>>>    agree that this probably should be rewritten better to make the
>>>    logic more obvious.  Maybe separate clauses for reflection/lambda
>>>    code?
>>> 
>>>        ((!is_MAI && !is_MLI) ||
>>>        (is_MAI && VerifyReflectionBytecodes) ||
>>>        (is_MLI && VerifyLambdaBytecodes))
>>> 
>>>    Isn't this equivalent to:
>>>      (!is_MAI || VerifyReflectionBytecodes) &&
>>>      (!is_MLI || VerifyLambdaBytecodes)
>>> 
>>> 
>>> 
>>>    On Wed, Aug 28, 2013 at 8:50 PM, Karen Kinnear
>>>    <Karen.Kinnear at oracle.com <mailto:Karen.Kinnear at oracle.com>> wrote:
>>> 
>>>        Thank you David. I agree, I was tempted to rewrite the entire
>>>        method - in particular, in a debugger
>>>        (so I didn't check the optimized code) we walk the entire &&
>>>        before we make the initial call - in this
>>>        case the initial call would weed out more things faster
>>>        potentially, so ... But I chose not to rewrite since
>>>        they need this quickly and the shaking out would take quite a
>>>        bit longer.
>>> 
>>>        thanks for the review,
>>>        Karen
>>> 
>>>        On Aug 28, 2013, at 8:48 PM, David Holmes wrote:
>>> 
>>>        > Hi Karen,
>>>        >
>>>        > I found the boolean logic very hard to follow there - not
>>>        sure if the refactoring helped or hindered :)
>>>        >
>>>        > But it looks okay.
>>>        >
>>>        > David
>>>        >
>>>        > On 29/08/2013 5:42 AM, Karen Kinnear wrote:
>>>        >> Please review - lambda team needs this change to get in by
>>>        tomorrow so
>>>        >> they can push
>>>        >> the (8010433) metafactory change which is waiting on the
>>>        vm. Thanks.
>>>        >>
>>>        >>
>>>        >> webrev: http://cr.openjdk.java.net/~acorn/8023872/webrev/
>>>        >> http://bugs.sun.com/view_bug.do?bug_id=8023872
>>>        >>
>>>        >> Testing:
>>>        >> The failure comes if you apply an early patch of 8010433 to
>>>        the jdk and
>>>        >> do not
>>>        >> make the vm change. So testing included 4 binaries: master,
>>>        newjdk,
>>>        >> newvm, newjdkvm
>>>        >>
>>>        >> jtreg for SpinedBufferTest with various flags - these are
>>>        the expected
>>>        >> results below
>>>        >> The key point is newjdk -Xverify: all fails
>>>        >> newjdkvm -Xverify:all passes, but if you also turn on
>>>        >> -XX:+VerifyLambdaBytecodes you see
>>>        >>    the verification error
>>>        >>
>>>        >> 1. master
>>>        >>  no flags: timed out
>>>        >>  -Xverify:all: timed out
>>>        >>  -XX:+VerifyReflectionBytecodes: VerifyError in reflection
>>>        >>
>>>        >> 2. newjdk
>>>        >>  no flags: timed out
>>>        >>  -Xverify:all
>>>        >>    java.lang.BootstrapMethodError: call site initialization
>>>        exception
>>>        >> ... cause:
>>>        >>      VerifyError: Bad invokespecial instruction: current
>>>        class isn't
>>>        >> assignable to reference class
>>>        >>  -XX:+VerifyReflectionBytecodes: VerifyError in reflection
>>>        >>
>>>        >> 3. newvm
>>>        >>   no flags: timed out
>>>        >>   rerun.sh: -Xverify:all: passed
>>>        >>   -XX:+VerifyReflectionBytecodes: VerifyError in reflection
>>>        >>   -XX:+VerifyLambdaBytecodes: passed, didn't try verification
>>>        >>   rerun.sh: -Xverify:all -XX:+VerifyLambdaBytecodes: passed
>>>        >>
>>>        >> 4. newjdkvm
>>>        >>  rerun.sh: no flags: passed
>>>        >> rerun.verify.sh <http://rerun.verify.sh/>: -Xverify:all: passed
>>>        >>  rerun.sh: -Xverify:all -XX:+VerifyLambdaBytecodes:
>>>        >>    Verification for
>>>        java.util.streamIntPipeline$7$1$$Lambda$9 failed
>>>        >>
>>>        >> regression testing:
>>>        >> newvm:
>>>        >>   vm.quick.testlist - in progress
>>>        >>   jtreg java/util/stream - in progress
>>>        >>   jprt -stree . - in progress
>>>        >>
>>>        >> newvmjdk (Brian testing in lambda repo) - in progress
>>>        >>
>>>        >> thanks,
>>>        >> Karen
>>>        >>
>>> 
>>> 
>>> 
>>> 
>>>    --
>>>    - Keith McGuigan <keith.mcguigan at alumni.unh.edu
>>>    <mailto:keith.mcguigan at alumni.unh.edu>>
>> 
>> 
>> 
>> 
>> --
>> - Keith McGuigan <keith.mcguigan at alumni.unh.edu
>> <mailto:keith.mcguigan at alumni.unh.edu>>