RFR(S) 8074010: followup to 8072383

[David Holmes]

Thanks Dean - looks good to me.

David

On 27/02/2015 4:32 PM, Dean Long wrote:
> Thanks David for looking at this.  Round two no longer tries to simplify
> the
> makefile logic, but instead keeps existing variables names and minimizes
> the number of lines changed.
>
> dl
>
> http://cr.openjdk.java.net/~dlong/8074010/8u60.01/
>
> On 2/26/2015 10:09 PM, David Holmes wrote:
>> Hi Dean,
>>
>> On 27/02/2015 3:32 PM, Dean Long wrote:
>>> This changeset removes references to closed platforms from gcc.make,
>>> allows $(HS_ALT_MAKE)/linux/makefiles/gcc.make to override the
>>> default values, and simplifies the logic a bit.  It is targeted for
>>> 8u60.
>>
>> For the benefit of other readers the problem with the existing code is
>> that we use platform specific variables to define an additional flag eg:
>>
>> DEBUG_CFLAGS/amd64 = -g
>>
>> which is then added to the primary flag variable ie:
>>
>> DEBUG_CFLAGS += $(DEBUG_CFLAGS/$(BUILDARCH))
>>
>> That is all good. The problem is the code used to set a default:
>>
>>    ifeq ($(DEBUG_CFLAGS/$(BUILDARCH)),)
>>        ifeq ($(USE_CLANG), true)
>>          # Clang doesn't understand -gstabs
>>          DEBUG_CFLAGS += -g
>>        else
>>          DEBUG_CFLAGS += -gstabs
>>        endif
>>    endif
>>
>> Here we check the current value of $(DEBUG_CFLAGS/$(BUILDARCH)) and
>> use it's emptiness to hard-wire a specific flag onto the primary flag
>> variable. The problem with that is when we include
>> $(HS_ALT_MAKE)/linux/makefiles/gcc.make at the end of the file, it
>> wants to set a value of DEBUG_CFLAGS/$(BUILDARCH) for another
>> BUILDARCH and consequently the main flags variable will get that value
>> AND the default that was hard-wired - and we don't won't that.
>>
>> However, a cleaner solution that Dean and I have discussed is to
>> simply adjust the default-setting as follows:
>>
>>    ifeq ($(DEBUG_CFLAGS/$(BUILDARCH)),)
>>        ifeq ($(USE_CLANG), true)
>>          # Clang doesn't understand -gstabs
>>          DEBUG_CFLAGS/$(BUILDARCH) = -g
>>        else
>>          DEBUG_CFLAGS/$(BUILDARCH) = -gstabs
>>        endif
>>    endif
>>
>> so now if we later set DEBUG_CFLAGS/$(BUILDARCH) for a hitherto unseen
>> BUILDARCH it will be expanded in the primary flags variable as
>> expected and the "default" will not be used.
>>
>> Note there's also an existing bug in setting of the FASTDEBUG_CFLAGS
>> which has since been fixed in 9 and is now fixed here as well:
>>
>> -    FASTDEBUG_CFLAGS += $(DEBUG_CFLAGS/$(BUILDARCH))
>> +    FASTDEBUG_CFLAGS += $(FASTDEBUG_CFLAGS/$(BUILDARCH))
>>
>> Thanks,
>> David
>>
>>> dl
>>>
>>> https://bugs.openjdk.java.net/browse/JDK-8074010
>>> http://cr.openjdk.java.net/~dlong/8074010/8u60.00/
>>>
>