RFR: 8313141: Remove discarded volatile qualifier in assembler_aarch64.hpp [v3]

[Kim Barrett]

On Fri, 28 Jul 2023 09:37:35 GMT, Andrew Haley <aph at openjdk.org> wrote:

>> Julian Waters has updated the pull request incrementally with two additional commits since the last revision:
>> 
>>  - assembler_aarch64.hpp
>>  - src/hotspot/cpu/aarch64/assembler_aarch64.hpp
>>    
>>    Co-authored-by: Andrew Haley <aph-open at littlepinkcloud.com>
>
> What is gained by this change? 
> 
> You've removed a volatile qualifier, which does no harm but indicates that an asm must be retained, and added a comment to that effect. The volatile qualifier doesn't have any affect because the compiler would have retained it anyway.
> 
> If I may use an analogy, using volatile in this context is like using a left-turn signal when in a left-turning lane. It doesn't do much good because you have to turn left from this lane, but it doesn't do any harm either. Many people always signal anyway.

I agree with @theRealAph. I think the volatile qualifier indicates intent to
the reader, even if the implementation doesn't require it. One needs to be
well-versed in the intricacies of gcc's asm extensions to know BasicAsm is
implicitly volatile; I'd certainly long ago forgotten that tidbit.

The existing comment isn't great.  But I think the replacement is confusing
and arguably wrong, since only the gnu BasicAsm extension provides that
feature, and not the gnu ExtendedAsm extension.  It's also not really related
to C++ - the C++ standard says almost nothing about asm other than being
implementaion-defined.

But is improving that comment really worth the time we've already spent on
this?  (Both here and in the other PR from which this was extracted.)

I'm going to just say no, don't make this change.

-------------

PR Comment: https://git.openjdk.org/jdk/pull/15031#issuecomment-1656471727