Request reviews (S): 6896370: CTW fails share/vm/opto/matcher.cpp:1475 "duplicating node that's already been matched"

[Tom Rodriguez]

Sorry for some reason I thought it didn't. Looks good then.

Tom

On Nov 4, 2009, at 12:45 PM, Vladimir Kozlov <Vladimir.Kozlov at Sun.COM>  
wrote:

>
>
> Tom Rodriguez wrote:
>> On Nov 4, 2009, at 10:43 AM, Vladimir Kozlov wrote:
>>>
>>> http://cr.openjdk.java.net/~kvn/6896370/webrev.00
>>>
>>> Fixed 6896370: CTW fails share/vm/opto/matcher.cpp:1475  
>>> "duplicating node that's already been matched"
>>>
>> I like this.  Which ones were missing?  There's also an oddity that
>
> LoadUB, LoadUI2L, LoadPLocked, LoadLLocked and all LoadStore nodes.
>
>> !is_Store() && is_Mem() != is_Load() so you're now treating  
>> LoadStore nodes as loads and mem_ops and they weren't previously.   
>> Was that intentional?  Calling set_shared on LoadStoreNodes is  
>> probably benign
>
> Yes, I did it intentionally since all Store[P|I|L]Conditional and
> CompareAndSwap nodes have general memory with all address modes.
>
>> but triggering the clone_shift_expressions logic for them probably  
>> isn't.  Most cas style instructions don't support full address  
>> modes so any cloning would be useless.
>
> I disagree, according to x86 documents cas uses general memory:
>
> CMPXCHG r/m32,r32    -    Compare EAX with r/m32. If equal, ZF is  
> set and r32 is
>                          loaded into r/m32. Else, clear ZF and load  
> r/m32 into AL
>
> Vladimir
>
>> tom
>>>
>>> Reviewed by:
>>>
>>> Fix verified (y/n): y, test
>>>
>>> Other testing:
>>> JPRT, CTW
>>>