[9] RFR (XS): 8036667: "assert(adr->is_AddP() && adr->in(AddPNode::Offset)->is_Con()) failed: offset is a constant" with FoldStableValues on

Vladimir Kozlov vladimir.kozlov at oracle.com
Thu Mar 6 09:47:47 PST 2014

Looks good.


On 3/6/14 9:40 AM, Vladimir Ivanov wrote:
> Unfortunately, replacing tp->offset() with AddPNode::Ideal_base_and_offset doesn't work for other cases in
> LoadNode::Value. I have to revert to using tp->offset().
> Updated webrev: http://cr.openjdk.java.net/~vlivanov/8036667/webrev.02/
> I did some testing and haven't found any problems with the adr being PhiNode, but I'm not sure I understand all aspects
> of this code. So I decided to skip phi case by adding adr->is_AddP() case. Some other cases in LoadNode::Value have
> similar check.
> Best regards,
> Vladimir Ivanov
> On 3/6/14 6:01 AM, Vladimir Ivanov wrote:
>>> Why you did changes in extract_uncommon_trap_request()? How we can get
>>> phi there?
>> It's a fix for a crash w/ -XX:+TraceIterGVN.
>> Will file a separate bug to track it, as you suggested.
>>> What is 'off' value in this case? How you can get constant value from C2
>>> type system (tp->offset()) but not from looking on ideal nodes?
>> Good point! My fix isn't correct in some cases.
>> In case only 1 phi input has non-top type, it should be safe to use
>> offset. But when at least 2 inputs are live, it's not correct (same
>> offset, but different address).
>> So, I need to ignore phi case completely.
>> Updated fix: http://cr.openjdk.java.net/~vlivanov/8036667/webrev.01/
>> Best regards,
>> Vladimir Ivanov
>>> On 3/5/14 4:26 PM, Vladimir Ivanov wrote:
>>>> http://cr.openjdk.java.net/~vlivanov/8036667/webrev.00/
>>>> https://bugs.openjdk.java.net/browse/JDK-8036667
>>>> 10 lines changed: 8 ins; 0 del; 2 mod
>>>> The assert is too strong. It doesn't take into account the case when
>>>> constant offset comes from phi node.
>>>> The fix is to relax the assertion by skipping phi node case. I decided
>>>> to keep it simple and avoided doing deep verification of the graph.
>>>> Also, fixed a crash w/ -XX:+TraceIterGVN (see callnode.cpp).
>>>> Testing: failing tests.
>>>> Thanks!
>>>> Best regards,
>>>> Vladimir Ivanov

More information about the hotspot-compiler-dev mailing list