S RFR: Lambda: verification error in generated lambda classes

[David Holmes]

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>>