Hello Patrick Georgi, Angel Pons,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/39633
to review the following change.
Change subject: util/xcompile: Split $CFLAGS_GCC ......................................................................
util/xcompile: Split $CFLAGS_GCC
Split common flags that are not specific to the C language out of $CFLAGS_GCC into $FLAGS_GCC. This way, we can test for C specific flags, too, without adding them to $ADAFLAGS_*. Currently this is done for `-Wno-address-of-packed-member` which only applies to C.
Change-Id: Ib793c62656efb07b6e5b3385f1ed1c96a40efd1d Signed-off-by: Nico Huber nico.h@gmx.de --- M util/xcompile/xcompile 1 file changed, 17 insertions(+), 17 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/33/39633/1
diff --git a/util/xcompile/xcompile b/util/xcompile/xcompile index 3203d71..18e08a0 100755 --- a/util/xcompile/xcompile +++ b/util/xcompile/xcompile @@ -148,11 +148,11 @@ [ "$obj_arch" = "$full_arch" ] || return 1
unset ASFLAGS LDFLAGS - unset CFLAGS_GCC CFLAGS_CLANG + unset FLAGS_GCC CFLAGS_GCC CFLAGS_CLANG
if [ -n "$use_dash_twidth" ]; then ASFLAGS="--$twidth" - CFLAGS_GCC="-m$twidth" + FLAGS_GCC="-m$twidth" CFLAGS_CLANG="-m$twidth" LDFLAGS="-b $full_arch"
@@ -162,7 +162,7 @@ [ -n "$use_dash_twidth" ] && case "$full_arch" in "elf32-i386" ) LDFLAGS="$LDFLAGS -melf_i386" - CFLAGS_GCC="$CFLAGS_GCC -Wl,-b,elf32-i386 -Wl,-melf_i386" + FLAGS_GCC="$FLAGS_GCC -Wl,-b,elf32-i386 -Wl,-melf_i386" CFLAGS_CLANG="$CFLAGS_CLANG -Wl,-b,elf32-i386 -Wl,-melf_i386" ;; esac @@ -173,19 +173,19 @@ detect_special_flags() { local architecture="$1" # Check for an operational -m32/-m64 - testcc "$GCC" "$CFLAGS_GCC -m$TWIDTH " && - CFLAGS_GCC="$CFLAGS_GCC -m$TWIDTH " + testcc "$GCC" "$FLAGS_GCC -m$TWIDTH " && + FLAGS_GCC="$FLAGS_GCC -m$TWIDTH "
# Use bfd linker instead of gold if available: - testcc "$GCC" "$CFLAGS_GCC -fuse-ld=bfd" && - CFLAGS_GCC="$CFLAGS_GCC -fuse-ld=bfd" && LINKER_SUFFIX='.bfd' + testcc "$GCC" "$FLAGS_GCC -fuse-ld=bfd" && + FLAGS_GCC="$FLAGS_GCC -fuse-ld=bfd" && LINKER_SUFFIX='.bfd'
- testcc "$GCC" "$CFLAGS_GCC -fno-stack-protector" && - CFLAGS_GCC="$CFLAGS_GCC -fno-stack-protector" - testcc "$GCC" "$CFLAGS_GCC -Wl,--build-id=none" && - CFLAGS_GCC="$CFLAGS_GCC -Wl,--build-id=none" + testcc "$GCC" "$FLAGS_GCC -fno-stack-protector" && + FLAGS_GCC="$FLAGS_GCC -fno-stack-protector" + testcc "$GCC" "$FLAGS_GCC -Wl,--build-id=none" && + FLAGS_GCC="$FLAGS_GCC -Wl,--build-id=none"
- testcc "$GCC" "$CFLAGS_GCC -Wno-address-of-packed-member" && + testcc "$GCC" "$CFLAGS_GCC -Wno-address-of-packed-member $FLAGS_GCC" && CFLAGS_GCC="$CFLAGS_GCC -Wno-address-of-packed-member" case "$architecture" in x86) @@ -193,7 +193,7 @@ x64) ;; arm64) - testld "$GCC" "$CFLAGS_GCC" "${GCCPREFIX}ld${LINKER_SUFFIX}" \ + testld "$GCC" "$FLAGS_GCC" "${GCCPREFIX}ld${LINKER_SUFFIX}" \ "$LDFLAGS --fix-cortex-a53-843419" && \ LDFLAGS_ARM64_A53_ERRATUM_843419+=" --fix-cortex-a53-843419" ;; @@ -202,7 +202,7 @@
detect_compiler_runtime() { test -z "$GCC" || \ - CC_RT_GCC="$(${GCC} ${CFLAGS_GCC} -print-libgcc-file-name)" + CC_RT_GCC="$(${GCC} ${CFLAGS_GCC} ${FLAGS_GCC} -print-libgcc-file-name)" if [ ${CLANG_RUNTIME} = "libgcc" ]; then CC_RT_CLANG=${CC_RT_GCC} else @@ -219,10 +219,10 @@
# GCC GCC_CC_${TARCH}:=${GCC} -GCC_CFLAGS_${TARCH}:=${CFLAGS_GCC} +GCC_CFLAGS_${TARCH}:=${CFLAGS_GCC} ${FLAGS_GCC} # Generally available for GCC's cc1: GCC_CFLAGS_${TARCH}+=-fno-delete-null-pointer-checks -Wlogical-op -GCC_ADAFLAGS_${TARCH}:=${CFLAGS_GCC} +GCC_ADAFLAGS_${TARCH}:=${FLAGS_GCC} GCC_COMPILER_RT_${TARCH}:=${CC_RT_GCC} GCC_COMPILER_RT_FLAGS_${TARCH}:=${CC_RT_EXTRA_GCC}
@@ -425,7 +425,7 @@ "" "$endian" || testas "$gccprefix" "$TWIDTH" "$TBFDARCH" \ "TRUE" "$endian" ; } && \ - testcc "${gccprefix}gcc" "$CFLAGS_GCC" && \ + testcc "${gccprefix}gcc" "$CFLAGS_GCC" "$FLAGS_GCC" && \ GCCPREFIX="$gccprefix" && \ break 3 done
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39633 )
Change subject: util/xcompile: Split $CFLAGS_GCC ......................................................................
Patch Set 1: Code-Review+1
HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39633 )
Change subject: util/xcompile: Split $CFLAGS_GCC ......................................................................
Patch Set 1: Code-Review+1
Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39633 )
Change subject: util/xcompile: Split $CFLAGS_GCC ......................................................................
Patch Set 1: Code-Review+2
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/39633 )
Change subject: util/xcompile: Split $CFLAGS_GCC ......................................................................
util/xcompile: Split $CFLAGS_GCC
Split common flags that are not specific to the C language out of $CFLAGS_GCC into $FLAGS_GCC. This way, we can test for C specific flags, too, without adding them to $ADAFLAGS_*. Currently this is done for `-Wno-address-of-packed-member` which only applies to C.
Change-Id: Ib793c62656efb07b6e5b3385f1ed1c96a40efd1d Signed-off-by: Nico Huber nico.h@gmx.de Reviewed-on: https://review.coreboot.org/c/coreboot/+/39633 Reviewed-by: Angel Pons th3fanbus@gmail.com Reviewed-by: HAOUAS Elyes ehaouas@noos.fr Reviewed-by: Patrick Georgi pgeorgi@google.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M util/xcompile/xcompile 1 file changed, 17 insertions(+), 17 deletions(-)
Approvals: build bot (Jenkins): Verified Patrick Georgi: Looks good to me, approved HAOUAS Elyes: Looks good to me, but someone else must approve Angel Pons: Looks good to me, but someone else must approve
diff --git a/util/xcompile/xcompile b/util/xcompile/xcompile index 3203d71..18e08a0 100755 --- a/util/xcompile/xcompile +++ b/util/xcompile/xcompile @@ -148,11 +148,11 @@ [ "$obj_arch" = "$full_arch" ] || return 1
unset ASFLAGS LDFLAGS - unset CFLAGS_GCC CFLAGS_CLANG + unset FLAGS_GCC CFLAGS_GCC CFLAGS_CLANG
if [ -n "$use_dash_twidth" ]; then ASFLAGS="--$twidth" - CFLAGS_GCC="-m$twidth" + FLAGS_GCC="-m$twidth" CFLAGS_CLANG="-m$twidth" LDFLAGS="-b $full_arch"
@@ -162,7 +162,7 @@ [ -n "$use_dash_twidth" ] && case "$full_arch" in "elf32-i386" ) LDFLAGS="$LDFLAGS -melf_i386" - CFLAGS_GCC="$CFLAGS_GCC -Wl,-b,elf32-i386 -Wl,-melf_i386" + FLAGS_GCC="$FLAGS_GCC -Wl,-b,elf32-i386 -Wl,-melf_i386" CFLAGS_CLANG="$CFLAGS_CLANG -Wl,-b,elf32-i386 -Wl,-melf_i386" ;; esac @@ -173,19 +173,19 @@ detect_special_flags() { local architecture="$1" # Check for an operational -m32/-m64 - testcc "$GCC" "$CFLAGS_GCC -m$TWIDTH " && - CFLAGS_GCC="$CFLAGS_GCC -m$TWIDTH " + testcc "$GCC" "$FLAGS_GCC -m$TWIDTH " && + FLAGS_GCC="$FLAGS_GCC -m$TWIDTH "
# Use bfd linker instead of gold if available: - testcc "$GCC" "$CFLAGS_GCC -fuse-ld=bfd" && - CFLAGS_GCC="$CFLAGS_GCC -fuse-ld=bfd" && LINKER_SUFFIX='.bfd' + testcc "$GCC" "$FLAGS_GCC -fuse-ld=bfd" && + FLAGS_GCC="$FLAGS_GCC -fuse-ld=bfd" && LINKER_SUFFIX='.bfd'
- testcc "$GCC" "$CFLAGS_GCC -fno-stack-protector" && - CFLAGS_GCC="$CFLAGS_GCC -fno-stack-protector" - testcc "$GCC" "$CFLAGS_GCC -Wl,--build-id=none" && - CFLAGS_GCC="$CFLAGS_GCC -Wl,--build-id=none" + testcc "$GCC" "$FLAGS_GCC -fno-stack-protector" && + FLAGS_GCC="$FLAGS_GCC -fno-stack-protector" + testcc "$GCC" "$FLAGS_GCC -Wl,--build-id=none" && + FLAGS_GCC="$FLAGS_GCC -Wl,--build-id=none"
- testcc "$GCC" "$CFLAGS_GCC -Wno-address-of-packed-member" && + testcc "$GCC" "$CFLAGS_GCC -Wno-address-of-packed-member $FLAGS_GCC" && CFLAGS_GCC="$CFLAGS_GCC -Wno-address-of-packed-member" case "$architecture" in x86) @@ -193,7 +193,7 @@ x64) ;; arm64) - testld "$GCC" "$CFLAGS_GCC" "${GCCPREFIX}ld${LINKER_SUFFIX}" \ + testld "$GCC" "$FLAGS_GCC" "${GCCPREFIX}ld${LINKER_SUFFIX}" \ "$LDFLAGS --fix-cortex-a53-843419" && \ LDFLAGS_ARM64_A53_ERRATUM_843419+=" --fix-cortex-a53-843419" ;; @@ -202,7 +202,7 @@
detect_compiler_runtime() { test -z "$GCC" || \ - CC_RT_GCC="$(${GCC} ${CFLAGS_GCC} -print-libgcc-file-name)" + CC_RT_GCC="$(${GCC} ${CFLAGS_GCC} ${FLAGS_GCC} -print-libgcc-file-name)" if [ ${CLANG_RUNTIME} = "libgcc" ]; then CC_RT_CLANG=${CC_RT_GCC} else @@ -219,10 +219,10 @@
# GCC GCC_CC_${TARCH}:=${GCC} -GCC_CFLAGS_${TARCH}:=${CFLAGS_GCC} +GCC_CFLAGS_${TARCH}:=${CFLAGS_GCC} ${FLAGS_GCC} # Generally available for GCC's cc1: GCC_CFLAGS_${TARCH}+=-fno-delete-null-pointer-checks -Wlogical-op -GCC_ADAFLAGS_${TARCH}:=${CFLAGS_GCC} +GCC_ADAFLAGS_${TARCH}:=${FLAGS_GCC} GCC_COMPILER_RT_${TARCH}:=${CC_RT_GCC} GCC_COMPILER_RT_FLAGS_${TARCH}:=${CC_RT_EXTRA_GCC}
@@ -425,7 +425,7 @@ "" "$endian" || testas "$gccprefix" "$TWIDTH" "$TBFDARCH" \ "TRUE" "$endian" ; } && \ - testcc "${gccprefix}gcc" "$CFLAGS_GCC" && \ + testcc "${gccprefix}gcc" "$CFLAGS_GCC" "$FLAGS_GCC" && \ GCCPREFIX="$gccprefix" && \ break 3 done
-
Angel Pons (Code Review) -
HAOUAS Elyes (Code Review)
-
Nico Huber (Code Review)
-
Patrick Georgi (Code Review)