On Jul 17, 2012, at 4:04 PM, Christian Thalinger wrote:
>>
>> I see in several files next code pattern. Should we call
>> throw_IncompatibleClassChangeError() as we do in other places?:
>>
>> + if (!EnableInvokeDynamic) {
>> + // rewriter does not generate this bytecode
>> + __ should_not_reach_here();
>> + return;
>> + }
>
> Hmm. This really should not happen and EnableInvokeDynamic is on by default
> anyway. I doubt someone turns it off.
It's now a diagnostic switch. It can be used to verify that an app. is not
using 292, which is occasionally helpful.
>> constantPoolOop.cpp:
>> Why not use guarantee() for bad operants?
>
> Not sure. John?
The code in that file uses assert; I think I'm following the existing practice
there. The CP code operates after initial verification passes, so assert is
suitable, I think.
>> Why you need separate scopes in resolve_bootstrap_specifier_at_impl()?
>
> To keep oops from being visible. Might result in bad crashes.
Yes. An alternative is to have the x_oop variables visible at top level, but
to put in DEBUG_ONLY(x_oop = NULL). Having the temporary scoped seems cleaner
to me, but maybe my sense of style is off here.
>
> I will refresh the review patch in mlvm. Thank you!
Yes, thanks!
— John
_______________________________________________
mlvm-dev mailing list
mlvm-dev@openjdk.java.net
http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev
