Name clashes - was: Re: [RFR]: 8186723: Add SuperH as new architecture for linux

[David Holmes]

On 25/08/2017 7:14 PM, John Paul Adrian Glaubitz wrote:
> On 08/25/2017 03:58 AM, David Holmes wrote:
>>> I agree. The string "LEGACY" in the identifier seems to confirm that. 
>>> Might be a good idea
>>> to open a separate bug for the removal of this identifier.
>>
>> Seems usage of these -D<ARCH> values are pervasive throughout the 
>> hotspot sources, so they won't be going away.
> 
> There are two variants though. One with uppercase and one with 
> lowercase. From what
> I can see, only the uppercase versions are only ever used, i.e. it's 
> "#ifdef(SPARC)"
> but never "#ifdef(sparc)".

Not clear where the lowercase one is coming from - I don't see it in a 
solaris sparc build log. ?? (I do see sparcv9 when building tests though)

David

>>>> Short-term solution would be your option (b) and rename alpha and sh 
>>>> to something unlikely to cause a clash.
>>> Will do.
>>>
>>> I guess we can merge 8186723 without any changes then? It doesn't 
>>> rely on the legacy
>>> (lowercase) identifier but on the all-uppercase identifier anyways.
>>
>> I guess so.
> 
> Ok. Then I guess we need a second reviewer?
> 
> Thomas, any concerns from your side?
> 
>> BTW it is odd that this:
>>
>>        #error Method os::dll_load requires that one of following is 
>> defined:\
>>           AARCH64, ALPHA, ARM, AMD64, IA32, IA64, M68K, MIPS, MIPSEL, 
>> PARISC, __powerpc__, __powerpc64__, S390, SH, __sparc
>>
>> is a mix of the configure generated -D<ARCH> values and what are 
>> presumably compiler defined ones. It should, for consistency, be using 
>> PPC32, PPC64 and SPARC. <sigh>
> 
> Yep, I noticed that as well. I can fix that after merging this one :-).
> 
> Adrian
>