RFR(S) 8074010: followup to 8072383
Dean Long
dean.long at oracle.com
Fri Feb 27 06:32:47 UTC 2015
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/
>>
More information about the hotspot-compiler-dev
mailing list