C1: fix "assert(has_printable_bci()) failed: _printable_bci should have been set" when using -XX:+PrintCanonicalization

[Christian Thalinger]

On May 29, 2012, at 6:55 PM, Kris Mok wrote:

> Hi Chris,
> 
> Thanks for looking at it. Could you sponsor this change, please?

7172843: C1: fix "assert(has_printable_bci()) failed: _printable_bci should have been set"

Will submit to JPRT shortly.

-- Chris

> 
> Thanks,
> Kris
> 
> On 2012-5-30, at 8:16, Christian Thalinger <christian.thalinger at oracle.com> wrote:
> 
>> Looks good.  Thanks for fixing this.  It hit me quite often while working on C1 and I never had time to fix it.
>> 
>> -- Chris
>> 
>> On May 29, 2012, at 12:10 PM, Krystal Mok wrote:
>> 
>>> Hi all,
>>> 
>>> Could I have a couple of review for this patch, please:
>>> https://gist.github.com/2829978#file_c1_fix_printable_bci.patch
>>> 
>>> I hit the "assert(has_printable_bci()) failed: _printable_bci should have been
>>> set" assertion when I was doing some experiment on C1, and needed to use
>>> -XX:+PrintCanonicalization for verification. It turns out that this flag is
>>> pretty broken, that quite a few places didn't set printable_bci appropriately.
>>> 
>>> This patch tries to fix the missing printable_bcis.
>>> The link also includes a simple example before and after applying this patch.
>>> 
>>> c1_Instructions.hpp:
>>> Added set_printable_bci() to Local and Phi's constructor.
>>> A "Local" instruction models an incoming argument, which gets its value before
>>> method entry, so I'm setting all Local's printable_bci to -1.
>>> A reasonable printable_bci for a "Phi" instruction is the same as the start bci
>>> of the basic block to which it belongs. I had to use a weird cast to get rid of
>>> a "invalid use of incomplete type 'struct BlockBegin' error from GCC.
>>> 
>>> c1_Canonicalizer.cpp:
>>> Added a default x->set_printable_bci(bci()) to Canonicalizer::set_canonical().
>>> There are quite a few place in Canonicalizer that doesn't specify the
>>> printable_bci for the newly created substitution instruction. It's reasonable
>>> to just set that to the "current" bci, which is the bci of the instruction to
>>> be substituted.
>>> Also adjusted the order of a set_bci() call in Canonicalizer::do_If, so that
>>> the new code above could pick up the modified bci in set_canonical().
>>> 
>>> Regards,
>>> Kris
>> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/attachments/20120530/93de142f/attachment-0001.html