RFR(M): 8024921: PPC64 (part 113): Extend Load and Store nodes to know about memory ordering.

[Vladimir Kozlov]

On 11/13/13 9:20 AM, Vitaly Davidovich wrote:
> Personally, I'd just use bool - you don't have a naming decision then :).
> You also won't need the asserts in accessor.

C2 usually uses enum for such cases. I would also prefer move enum to MemNode class (additional field should stay in 
subclasses) and list all values:

typedef enum { unordered = 0, acquire, release } MemOrder;

>
> If you stick with enum, how about MemOrder? Is that too long? You're only
> going to use that name in a few places so I think clarity trumps super
> short name.

+1

Thanks,
Vladimir

>
> Sent from my phone
> On Nov 13, 2013 11:54 AM, "Lindenmaier, Goetz" <goetz.lindenmaier at sap.com>
> wrote:
>
>>   Hi Vitaly,
>>
>>
>>
>> we use an enum so that we have a speaking name for the value.
>>
>> Sure we could use a ‘final bool unordered = false’ but enum seems
>>
>> the proper way.
>>
>>
>>
>> Propose a better name for Sem, I don’t like it too much either!
>>
>> I just used the name in our code (done by one of my former colleagues
>>
>> A few years ago).  I will change it if there is a better (and please
>> short) one.
>>
>>
>>
>> Best regards,
>>
>>    Goetz.
>>
>>
>>
>>
>>
>>
>>
>> *From:* Vitaly Davidovich [mailto:vitalyd at gmail.com]
>> *Sent:* Mittwoch, 13. November 2013 02:31
>> *To:* Lindenmaier, Goetz
>> *Cc:* Vladimir Kozlov; hotspot-dev developers;
>> ppc-aix-port-dev at openjdk.java.net
>> *Subject:* RE: RFR(M): 8024921: PPC64 (part 113): Extend Load and Store
>> nodes to know about memory ordering.
>>
>>
>>
>> Hi Goetz,
>>
>> Just curious - why an enum for both versions (load, store) and not a
>> simple bool? You expecting more values? You wouldn't need those asserts in
>> the accessors if this was a bool.
>>
>> Also what does "Sem" stand for? Semantic? Seems like an odd abbreviation.
>>
>> Thanks
>>
>> Sent from my phone
>>
>> On Nov 12, 2013 11:31 AM, "Lindenmaier, Goetz" <goetz.lindenmaier at sap.com>
>> wrote:
>>
>> Hi,
>>
>> I updated this webrev to work with the latest version of the staging
>> repository.
>> http://cr.openjdk.java.net/~goetz/webrevs/8024921-0-ldst/
>>
>> Best regards,
>>    Goetz.
>>
>> From: goetz.lindenmaier at sap.com
>> Sent: Freitag, 11. Oktober 2013 15:34
>> To: hotspot-dev developers; ppc-aix-port-dev at openjdk.java.net; Vladimir
>> Kozlov
>> Subject: RFR(M): 8024921: PPC64 (part 113): Extend Load and Store nodes to
>> know about memory ordering.
>>
>> Hi,
>>
>> I prepared a webrev for 8024921<
>> https://bugs.openjdk.java.net/browse/JDK-8024921>Extend Load and Store
>> nodes to know about memory ordering.
>> This is part of the PPC port.
>> http://cr.openjdk.java.net/~goetz/webrevs/8024921-0-ldst/
>>
>> For a detailed description see the text in the webrev and bug description.
>>
>> All this basically does is add a field to load and store nodes and
>> change all constructor calls to set this field.  So the effect on
>> existing platforms should be very small.  Therefore I marked this
>> 'M', although quite some lines of code are touched.
>>
>> Please review and test this change.
>> I'm happy to incorporate your comments and any improvements
>> you propose.
>>
>> Best regards,
>>    Goetz.
>>
>>
>>
>>