S RFR: Lambda: verification error in generated lambda classes

[Karen Kinnear]

Thanks Keith - I'll remove them. Thanks for the suggestion and the review,
Karen

On Aug 29, 2013, at 2:31 PM, Keith McGuigan wrote:

> I think there's an extra set of parenthesis that isn't needed, but other than that, thanks, looks good!
> 
> 
> On Thu, Aug 29, 2013 at 1:29 PM, Karen Kinnear <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> 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: -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>
> 
> 
> 
> 
> -- 
> - Keith McGuigan <keith.mcguigan at alumni.unh.edu>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/attachments/20130830/fccf298b/attachment-0001.html