RFR(S) 8074010: followup to 8072383 (original) (raw)

Vladimir Kozlov vladimir.kozlov at oracle.com
Tue Mar 3 20:13:38 UTC 2015

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 $(HSALTMAKE)/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: DEBUGCFLAGS/amd64 = -g which is then added to the primary flag variable ie: DEBUGCFLAGS += (DEBUGCFLAGS/(DEBUGCFLAGS/(DEBUGCFLAGS/(BUILDARCH)) That is all good. The problem is the code used to set a default: ifeq ($(DEBUGCFLAGS/$(BUILDARCH)),) ifeq ($(USECLANG), true) # Clang doesn't understand -gstabs DEBUGCFLAGS += -g else DEBUGCFLAGS += -gstabs endif endif Here we check the current value of (DEBUGCFLAGS/(DEBUGCFLAGS/(DEBUGCFLAGS/(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 $(HSALTMAKE)/linux/makefiles/gcc.make at the end of the file, it wants to set a value of DEBUGCFLAGS/$(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 ($(DEBUGCFLAGS/$(BUILDARCH)),) ifeq ($(USECLANG), true) # Clang doesn't understand -gstabs DEBUGCFLAGS/$(BUILDARCH) = -g else DEBUGCFLAGS/$(BUILDARCH) = -gstabs endif endif so now if we later set DEBUGCFLAGS/$(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 FASTDEBUGCFLAGS which has since been fixed in 9 and is now fixed here as well: - FASTDEBUGCFLAGS += (DEBUGCFLAGS/(DEBUGCFLAGS/(DEBUGCFLAGS/(BUILDARCH)) + FASTDEBUGCFLAGS += (FASTDEBUGCFLAGS/(FASTDEBUGCFLAGS/(FASTDEBUGCFLAGS/(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