RFR(S) 8074010: followup to 8072383

[Dean Long]

Ping.  I think I still need another review for this.

dl

On 2/27/2015 10:11 AM, Dean Long wrote:
> Thanks for the review, David.  Can I get another review for this, please?
>
> dl
>
> On 2/27/2015 12:43 AM, David Holmes wrote:
>> 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/
>>>>>
>>>
>