RFR(S) 8074010: followup to 8072383

[Vladimir Kozlov]

Good.

Thanks,
Vladimir

On 3/3/15 11:54 AM, Dean Long wrote:
> 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/
>>>>>>
>>>>
>>
>