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

[Vladimir Ivanov]

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