[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Minios-devel] [UNIKRAFT PATCHv4 07/43] build: Add a makefile function to check and set flags for valid gcc



On 06.07.2018 11:03, Wei Chen wrote:
Some times, we will add special flags to GCC to do optimization. For
instance, we can use this function to check valid GCC and set flags
to do processors optimization. In order to avoid any definition
clashes, we define this function in support/build/Makefile.rules.

Signed-off-by: Wei Chen <Wei.Chen@xxxxxxx>
---
  support/build/Makefile.rules | 8 ++++++++
  1 file changed, 8 insertions(+)

diff --git a/support/build/Makefile.rules b/support/build/Makefile.rules
index f71fd6e..261b8f5 100644
--- a/support/build/Makefile.rules
+++ b/support/build/Makefile.rules
@@ -42,6 +42,14 @@ $(if $(call test_gcc_version,$(1),$(2)),,\
       $(error Require GCC version >= $(1).$(2) found 
$(CC_VER_MAJOR).$(CC_VER_MINOR)))
  endef
+# set_flags_if_gcc_version_ge $gcc_major,$gcc_minor,$flags
+define set_flags_if_gcc_version_ge =
+$(call error_if_gcc_version_lt,$(1),$(2))

Oh, I would not expect from the set_flags_if_gcc_version_ge function name that it is going to let Make stop execution and returning an error. I would expect that it sets only the flags if the condition is met and ignore it otherwise. Hum...

+ASFLAGS-$(1)  += $(3)
+CFLAGS-$(1)   += $(3)
+CXXFLAGS-$(1) += $(3)

I had a chat with Sharan and we actually prefer forgetting about a function that sets flags to all these three variables. We get the feeling that the function is too powerful with the cost not to understand anymore what it does.

Instead of doing this:

$(call set_flags_if_gcc_version_ge,4,5,-my_flag)

...we think about a usage flow like this (as equal expression):

$(call error_if_gcc_version_lt,4,5)

ASFLAGS-$(call gcc_version_ge,4,5)  += -my_flag
CFLAGS-$(call gcc_version_ge,4,5)   += -my_flag
CXXFLAGS-$(call gcc_version_ge,4,5) += -my_flag

I know you have to write more for this but we think this is easier to get without needing to lookup what the Make functions actually do. gcc_version_ge is the function you have called test_gcc_version in your previous patch and which result in a `y` if the GCC version is greater or equal to the parameters.

I am sorry to suggest you again another change of this. I know that we agreed on the system you implemented right now. What do you think?


+endef
+
  
################################################################################
  #
  # Paths and Filenames


Thanks,

Simon

_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.