diff mbox series

'gcc/config/nvptx/t-nvptx': Don't use the 'shell' function of 'make' (was: nvptx: Allow '--with-arch' to override the default '-misa' (was: nvptx multilib setup))

Message ID 87jzcdmbh9.fsf@euler.schwinge.ddns.net
State New
Headers show
Series 'gcc/config/nvptx/t-nvptx': Don't use the 'shell' function of 'make' (was: nvptx: Allow '--with-arch' to override the default '-misa' (was: nvptx multilib setup)) | expand

Commit Message

Thomas Schwinge Dec. 6, 2024, 9:01 a.m. UTC 
Hi!

I recently learned that the exit status of the command invoked in a
'Makefile' via '$(shell [...])' effectively gets discarded (unless
explicitly checking the GNU Make 4.2+ '.SHELLSTATUS' variable or jumping
through other hoops).  I was under the assumption that an error in a
'shell' function would cause 'make' to error out, similarly to how it
does in 'Makefile' rules...

I learned this The Hard Way here:

On 2022-06-15T23:18:10+0200, I wrote:
> --- a/gcc/config/nvptx/t-nvptx
> +++ b/gcc/config/nvptx/t-nvptx

> +multilib_matches := $(shell $(srcdir)/config/nvptx/gen-multilib-matches.sh $(srcdir)/config/nvptx $(multilib_options_isa_default) "$(multilib_options_isa_list)")

When recently working on changing nvptx multilib things, and for that
enhancing nvptx' 'gen-multilib-matches.sh', I made an error in there, and
then got confusing behavior in that I could still successfully 'make'
GCC, and my changes "mostly appeared to work as expected", but not quite.
This was due to garbage in 'MULTILIB_MATCHES', caused by a shell syntax
error in 'gen-multilib-matches.sh' -- which '$(shell [...])' swept under
the table.

Pushed to trunk branch commit 490443357668a87e3c322f218873a7649a2552df
"'gcc/config/nvptx/t-nvptx': Don't use the 'shell' function of 'make'",
see attached.


Grüße
 Thomas
diff mbox series

Patch

From 490443357668a87e3c322f218873a7649a2552df Mon Sep 17 00:00:00 2001
From: Thomas Schwinge <tschwinge@baylibre.com>
Date: Mon, 2 Dec 2024 15:06:58 +0100
Subject: [PATCH] 'gcc/config/nvptx/t-nvptx': Don't use the 'shell' function of
 'make'

The exit status of the command invoked in a 'Makefile' via '$(shell [...])'
effectively gets discarded (unless explicitly checking the GNU Make 4.2+
'.SHELLSTATUS' variable or jumping through other hoops).  In order to be able
to catch errors in what the 'shell' function invokes, let's make things
explicit: similar to how 'gcc/config/avr/t-avr' is doing with 't-multilib-avr',
for example.

	gcc/
	* config/nvptx/t-nvptx (multilib_matches): Don't use the 'shell'
	function of 'make'.
	* config/nvptx/gen-multilib-matches.sh: Adjust.
---
 gcc/config/nvptx/gen-multilib-matches.sh |  9 +++++++--
 gcc/config/nvptx/t-nvptx                 | 14 +++++++++++++-
 2 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/gcc/config/nvptx/gen-multilib-matches.sh b/gcc/config/nvptx/gen-multilib-matches.sh
index 44c758c3b1bf..a39baee5cd24 100755
--- a/gcc/config/nvptx/gen-multilib-matches.sh
+++ b/gcc/config/nvptx/gen-multilib-matches.sh
@@ -33,6 +33,8 @@  sms=$(grep ^NVPTX_SM $nvptx_sm_def | sed 's/.*(//;s/,.*//')
 # ('misa=sm_SM'; thus not remapped), or has to be remapped to the "next lower"
 # variant that does get built.
 
+multilib_matches=
+
 # The "lowest" variant has to be built.
 sm_next_lower=INVALID
 
@@ -50,11 +52,14 @@  for sm in $sms; do
     else
 	# Output format as required for 'MULTILIB_MATCHES'.
 	if [ x"$sm_map" = x. ]; then
-	    echo ".=misa?sm_$sm"
+	    multilib_matches_sm=".=misa?sm_$sm"
 	else
-	    echo "misa?sm_$sm_map=misa?sm_$sm"
+	    multilib_matches_sm="misa?sm_$sm_map=misa?sm_$sm"
 	fi
+	multilib_matches="$multilib_matches $multilib_matches_sm"
 
 	sm_next_lower=$sm_map
     fi
 done
+
+echo "multilib_matches := $multilib_matches"
diff --git a/gcc/config/nvptx/t-nvptx b/gcc/config/nvptx/t-nvptx
index 9c5cbda00707..6c6a6329f0f8 100644
--- a/gcc/config/nvptx/t-nvptx
+++ b/gcc/config/nvptx/t-nvptx
@@ -43,12 +43,24 @@  MULTILIB_OPTIONS += mgomp
 multilib_options_isa_list := $(TM_MULTILIB_CONFIG)
 multilib_options_isa_default := $(word 1,$(multilib_options_isa_list))
 multilib_options_misa_list := $(addprefix misa=,$(multilib_options_isa_list))
+
+t-nvptx-gen-multilib-matches: $(srcdir)/config/nvptx/gen-multilib-matches.sh \
+  $(srcdir)/config/nvptx/t-nvptx \
+  Makefile \
+  $(srcdir)/config/nvptx/nvptx-sm.def
+	$(SHELL) $< \
+	  $(dir $<) \
+	  $(multilib_options_isa_default) \
+	  '$(multilib_options_isa_list)' \
+	  > $@
+
+include t-nvptx-gen-multilib-matches
+
 # Add the requested '-misa' variants as a multilib option ('misa=VAR1/misa=VAR2/misa=VAR3' etc.):
 empty :=
 space := $(empty) $(empty)
 MULTILIB_OPTIONS += $(subst $(space),/,$(multilib_options_misa_list))
 # ..., and remap '-misa' variants as appropriate:
-multilib_matches := $(shell $(srcdir)/config/nvptx/gen-multilib-matches.sh $(srcdir)/config/nvptx $(multilib_options_isa_default) "$(multilib_options_isa_list)")
 MULTILIB_MATCHES += $(multilib_matches)
 # ..., and don't actually build what's the default '-misa':
 MULTILIB_EXCEPTIONS += *misa=$(multilib_options_isa_default)*
-- 
2.34.1