RFR(XS) 8025657: compiler/intrinsics/mathexact/ConstantTest.java fails on assert in lcm.cpp on solaris x64

[Vladimir Kozlov]

Rickard,

Your code is correct. What I am trying to understand is it possible to 
make it more general and not just hack for mathExact.
Also if we set control edge to addExact node (for example, during 
final_graph_reshape). And make its machnode pinned (in adlc 
InstructForm::is_pinned()). Will current gcm code work then?

Speaking of cotrol edge for these nodes. You allow gvn these nodes (you 
have hash() method). But is this correct? What happens for duplicated 
code when inputs are the same?

addExact(x,y)
addExact(x,y)

Thanks,
Vladimir

On 10/6/13 10:13 PM, Rickard Bäckman wrote:
> Vladimir,
>
> both addExact and FlagsProj are in must_clone. It means they have to be close to use, but since there are 2
> uses of addExact this seems to be problematic.
>
> The code in schedule_pinned_nodes takes the addExact and FlagsProj and schedules them into the same block as the jump on overflow. It seems correct to me.
>
> Thank you
> /R
>
> On Oct 4, 2013, at 8:55 PM, Vladimir Kozlov wrote:
>
>> Rickard,
>>
>> addExact, should in must_clone[] list since it produce flag (I think your original changes have it). With that it should be scheduled near use (see PhaseCFG::schedule_late()). The problem could be it has 2 uses.
>>
>> FlagsProj should be scheduled near consumer of the flag (since it is also in must_clone[]). Otherwise flags state could be changes by other instructions. So moving it to addExact block is wrong. It should be opposite: addExact should at the same block as FlagsProj and conditional jump.
>>
>> Vladimir
>>
>> On 10/4/13 4:35 AM, Rickard Bäckman wrote:
>>> Hi all,
>>>
>>> I would appreciate some reviews on this change.
>>>
>>> On some machines when running with -XX:+DeoptimizeALot a test (test/compiler/intrinsics/mathexact/ConstantTest.java) failed with an assert.
>>> assert(idx >= 0) failed: index should be set
>>>
>>> It seems that gcm would put the addExact in a different block then the jump.
>>> This change forces the producer of FlagsProjNode to be in the same block as
>>> the consumer.
>>>
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8025657
>>> Webrev: http://cr.openjdk.java.net/~rbackman/8025657.1/
>>>
>>> Thanks
>>> /R
>>>
>