[v4,1/5] mk: introduce helper to check valid compiler argument

Message ID 20190109103915.29210-1-pbhagavatula@marvell.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series [v4,1/5] mk: introduce helper to check valid compiler argument |

Checks

Context Check Description
ci/mellanox-Performance-Testing success Performance Testing PASS
ci/intel-Performance-Testing success Performance Testing PASS
ci/Intel-compilation success Compilation OK

Commit Message

Pavan Nikhilesh Bhagavatula Jan. 9, 2019, 10:39 a.m. UTC
  From: Jerin Jacob <jerinj@marvell.com>

Introduce rte_cc_has_argument() Makefile helper to
check a given argument is support by the compiler.

Example Usage:

include $(RTE_SDK)/mk/rte.helper.mk
MACHINE_CFLAGS += $(call rte_cc_has_argument, -mcpu=octeontx2)

This would allow adding -mcpu=octeontx2 in MACHINE_CFLAGS
if it is only supported by the compiler. The use case for such
scheme is to enable the mcpu optimization if the compiler
supports else it needs to compile the source code without
any errors.

This patch also moves inclusion of toolchain's rte.vars.mk
to before the machine's rte.vars.mk inclusion to make
correct CC available for the cross compile case.

Signed-off-by: Jerin Jacob <jerinj@marvell.com>
Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
---

 v2 Changes:
 - Add meson build support.

 v3 Changes:
 - Squash meson build support into config support for thunderx2/octeontx2.

 v4 Changes:
 - Fix incorrect signoff marrvell -> marvell.

 mk/rte.helper.mk              | 12 ++++++++++++
 mk/target/generic/rte.vars.mk | 22 +++++++++++-----------
 2 files changed, 23 insertions(+), 11 deletions(-)
 create mode 100644 mk/rte.helper.mk

--
2.20.1
  

Comments

Thomas Monjalon Jan. 14, 2019, 11:35 a.m. UTC | #1
09/01/2019 11:39, Pavan Nikhilesh Bhagavatula:
> From: Jerin Jacob <jerinj@marvell.com>
> 
> Introduce rte_cc_has_argument() Makefile helper to
> check a given argument is support by the compiler.
> 
> Example Usage:
> 
> include $(RTE_SDK)/mk/rte.helper.mk
> MACHINE_CFLAGS += $(call rte_cc_has_argument, -mcpu=octeontx2)
> 
> This would allow adding -mcpu=octeontx2 in MACHINE_CFLAGS
> if it is only supported by the compiler. The use case for such
> scheme is to enable the mcpu optimization if the compiler
> supports else it needs to compile the source code without
> any errors.
> 
> This patch also moves inclusion of toolchain's rte.vars.mk
> to before the machine's rte.vars.mk inclusion to make
> correct CC available for the cross compile case.
> 
> Signed-off-by: Jerin Jacob <jerinj@marvell.com>
> Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
> ---
> --- /dev/null
> +++ b/mk/rte.helper.mk
> @@ -0,0 +1,12 @@
> +# SPDX-License-Identifier: BSD-3-Clause
> +# Copyright(c) 2018 Marvell International Ltd
> +
> +# rte_cc_has_argument
> +# Usage: MACHINE_CFLAGS += $(call rte_cc_has_argument, -mno-avx512f)
> +# Return the argument if the argument is supported by the compiler.
> +#
> +define rte_cc_has_argument
> +	$(shell $(CC) -Werror $(1) -c -x c /dev/null -o tmp$$ 2> /dev/null && rm -f tmp$$ && echo $(1) | xargs echo -n)
> +endef

What is tmp$$ ?

If the command is interrupted in the middle, temp file is not cleaned.
We could fix it with "trap".
Is it possible to just avoid creating a temporary file?
  
Jerin Jacob Kollanukkaran Jan. 14, 2019, 11:56 a.m. UTC | #2
On Mon, 2019-01-14 at 12:35 +0100, Thomas Monjalon wrote:
> -------------------------------------------------------------------
> ---
> 09/01/2019 11:39, Pavan Nikhilesh Bhagavatula:
> > From: Jerin Jacob <jerinj@marvell.com>
> > 
> > Introduce rte_cc_has_argument() Makefile helper to
> > check a given argument is support by the compiler.
> > 
> > Example Usage:
> > 
> > include $(RTE_SDK)/mk/rte.helper.mk
> > MACHINE_CFLAGS += $(call rte_cc_has_argument, -mcpu=octeontx2)
> > 
> > This would allow adding -mcpu=octeontx2 in MACHINE_CFLAGS
> > if it is only supported by the compiler. The use case for such
> > scheme is to enable the mcpu optimization if the compiler
> > supports else it needs to compile the source code without
> > any errors.
> > 
> > This patch also moves inclusion of toolchain's rte.vars.mk
> > to before the machine's rte.vars.mk inclusion to make
> > correct CC available for the cross compile case.
> > 
> > Signed-off-by: Jerin Jacob <jerinj@marvell.com>
> > Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
> > ---
> > --- /dev/null
> > +++ b/mk/rte.helper.mk
> > @@ -0,0 +1,12 @@
> > +# SPDX-License-Identifier: BSD-3-Clause
> > +# Copyright(c) 2018 Marvell International Ltd
> > +
> > +# rte_cc_has_argument
> > +# Usage: MACHINE_CFLAGS += $(call rte_cc_has_argument, -mno-
> > avx512f)
> > +# Return the argument if the argument is supported by the
> > compiler.
> > +#
> > +define rte_cc_has_argument
> > +	$(shell $(CC) -Werror $(1) -c -x c /dev/null -o tmp$$ 2>
> > /dev/null && rm -f tmp$$ && echo $(1) | xargs echo -n)
> > +endef
> 
> What is tmp$$ ?

It is created per process with pid value.

> 
> If the command is interrupted in the middle, temp file is not
> cleaned.

Yes. I think we can move to RTE_OUTPUT. Even it is not cleaned 
then it is file, I think, that would be easiest solution.

> We could fix it with "trap".

Can we do it in Makefile? 

> Is it possible to just avoid creating a temporary file?

I tried it but gcc creates one.

> 
> 
>
  
Thomas Monjalon Jan. 14, 2019, 12:08 p.m. UTC | #3
14/01/2019 12:56, Jerin Jacob Kollanukkaran:
> On Mon, 2019-01-14 at 12:35 +0100, Thomas Monjalon wrote:
> > -------------------------------------------------------------------
> > ---
> > 09/01/2019 11:39, Pavan Nikhilesh Bhagavatula:
> > > From: Jerin Jacob <jerinj@marvell.com>
> > > 
> > > Introduce rte_cc_has_argument() Makefile helper to
> > > check a given argument is support by the compiler.
> > > 
> > > Example Usage:
> > > 
> > > include $(RTE_SDK)/mk/rte.helper.mk
> > > MACHINE_CFLAGS += $(call rte_cc_has_argument, -mcpu=octeontx2)
> > > 
> > > This would allow adding -mcpu=octeontx2 in MACHINE_CFLAGS
> > > if it is only supported by the compiler. The use case for such
> > > scheme is to enable the mcpu optimization if the compiler
> > > supports else it needs to compile the source code without
> > > any errors.
> > > 
> > > This patch also moves inclusion of toolchain's rte.vars.mk
> > > to before the machine's rte.vars.mk inclusion to make
> > > correct CC available for the cross compile case.
> > > 
> > > Signed-off-by: Jerin Jacob <jerinj@marvell.com>
> > > Signed-off-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
> > > ---
> > > --- /dev/null
> > > +++ b/mk/rte.helper.mk
> > > @@ -0,0 +1,12 @@
> > > +# SPDX-License-Identifier: BSD-3-Clause
> > > +# Copyright(c) 2018 Marvell International Ltd
> > > +
> > > +# rte_cc_has_argument
> > > +# Usage: MACHINE_CFLAGS += $(call rte_cc_has_argument, -mno-
> > > avx512f)
> > > +# Return the argument if the argument is supported by the
> > > compiler.
> > > +#
> > > +define rte_cc_has_argument
> > > +	$(shell $(CC) -Werror $(1) -c -x c /dev/null -o tmp$$ 2>
> > > /dev/null && rm -f tmp$$ && echo $(1) | xargs echo -n)
> > > +endef
> > 
> > What is tmp$$ ?
> 
> It is created per process with pid value.

I see. The file is in current directory with name tmp + PID.

> > If the command is interrupted in the middle, temp file is not
> > cleaned.
> 
> Yes. I think we can move to RTE_OUTPUT. Even it is not cleaned 
> then it is file, I think, that would be easiest solution.
> 
> > We could fix it with "trap".
> 
> Can we do it in Makefile? 

Yes, it is just one more command separated with ;

> > Is it possible to just avoid creating a temporary file?
> 
> I tried it but gcc creates one.

It does not create a file in my test.

A better command is:
	$(CC) -E $(1) -xc /dev/null >/dev/null
Then you can just check the return value.

If you want rte_cc_has_argument returns a string or empty as true/false,
you can do this:
	$(CC) -E $(1) -xc /dev/null >/dev/null && echo $(1)
  

Patch

diff --git a/mk/rte.helper.mk b/mk/rte.helper.mk
new file mode 100644
index 000000000..2c5d5275e
--- /dev/null
+++ b/mk/rte.helper.mk
@@ -0,0 +1,12 @@ 
+# SPDX-License-Identifier: BSD-3-Clause
+# Copyright(c) 2018 Marvell International Ltd
+
+# rte_cc_has_argument
+# Usage: MACHINE_CFLAGS += $(call rte_cc_has_argument, -mno-avx512f)
+# Return the argument if the argument is supported by the compiler.
+#
+define rte_cc_has_argument
+	$(shell $(CC) -Werror $(1) -c -x c /dev/null -o tmp$$ 2> /dev/null && rm -f tmp$$ && echo $(1) | xargs echo -n)
+endef
+
+
diff --git a/mk/target/generic/rte.vars.mk b/mk/target/generic/rte.vars.mk
index dd149acc9..25a578ad7 100644
--- a/mk/target/generic/rte.vars.mk
+++ b/mk/target/generic/rte.vars.mk
@@ -7,6 +7,17 @@ 
 # executive environment.
 #

+#
+# toolchain:
+#
+#   - define CC, LD, AR, AS, ...
+#   - define TOOLCHAIN_CFLAGS variable (overridden by cmdline value)
+#   - define TOOLCHAIN_LDFLAGS variable (overridden by cmdline value)
+#   - define TOOLCHAIN_ASFLAGS variable (overridden by cmdline value)
+#   - may override any previously defined variable
+#
+include $(RTE_SDK)/mk/toolchain/$(RTE_TOOLCHAIN)/rte.vars.mk
+
 #
 # machine:
 #
@@ -45,17 +56,6 @@  endif
 #
 include $(RTE_SDK)/mk/arch/$(RTE_ARCH)/rte.vars.mk

-#
-# toolchain:
-#
-#   - define CC, LD, AR, AS, ...
-#   - define TOOLCHAIN_CFLAGS variable (overridden by cmdline value)
-#   - define TOOLCHAIN_LDFLAGS variable (overridden by cmdline value)
-#   - define TOOLCHAIN_ASFLAGS variable (overridden by cmdline value)
-#   - may override any previously defined variable
-#
-include $(RTE_SDK)/mk/toolchain/$(RTE_TOOLCHAIN)/rte.vars.mk
-
 #
 # exec-env:
 #