Change in coreboot[master]: xcompile: Use GCC wrappers for ar, nm

[Jacob Garber (Code Review)]

Jacob Garber has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/38290 )

Change subject: xcompile: Use GCC wrappers for ar, nm
......................................................................

xcompile: Use GCC wrappers for ar, nm

These wrappers provide the path to the plugin that ar and nm need to
understand LTO object files.

Change-Id: Ibdae4faabf67bf6a4bb8c38970f6189646ee74b3
Signed-off-by: Jacob Garber 
---
M util/xcompile/xcompile
1 file changed, 2 insertions(+), 2 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/90/38290/1

diff --git a/util/xcompile/xcompile b/util/xcompile/xcompile
index cc4641e..987789f 100755
--- a/util/xcompile/xcompile
+++ b/util/xcompile/xcompile
@@ -294,12 +294,12 @@
    fi # if [ "${TARCH}" = "arm64" ]...

cat <<EOF
-NM_${TARCH}:=${GCCPREFIX}nm
+NM_${TARCH}:=${GCCPREFIX}gcc-nm
 OBJCOPY_${TARCH}:=${GCCPREFIX}objcopy
 OBJDUMP_${TARCH}:=${GCCPREFIX}objdump
 READELF_${TARCH}:=${GCCPREFIX}readelf
 STRIP_${TARCH}:=${GCCPREFIX}strip
-AR_${TARCH}:=${GCCPREFIX}ar
+AR_${TARCH}:=${GCCPREFIX}gcc-ar
 GNATBIND_${TARCH}:=${GCCPREFIX}gnatbind
 CROSS_COMPILE_${TARCH}:=${GCCPREFIX}

[HAOUAS Elyes (Code Review)]

HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38290 )

Change subject: xcompile: Use GCC wrappers for ar, nm
......................................................................

Patch Set 2: Code-Review+1

[David Hendricks (Code Review)]

David Hendricks has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38290 )

Change subject: xcompile: Use GCC wrappers for ar, nm
......................................................................

Patch Set 3: Code-Review+1

This seems a bit strange, but based on a few minutes of searching around it appears to be correct... The sole purpose of these wrappers appears to be to add an LTO plugin [1], though the man page is much more generalized/cryptic.

It should have no impact on builds that do not use LTO, right?

[1] https://gcc.gnu.org/ml/gcc-help/2012-03/msg00100.html

[HAOUAS Elyes (Code Review)]

HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38290 )

Change subject: xcompile: Use GCC wrappers for ar, nm
......................................................................

Patch Set 3: Code-Review+2

[David Hendricks (Code Review)]

David Hendricks has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38290 )

Change subject: xcompile: Use GCC wrappers for ar, nm
......................................................................

Patch Set 3: Code-Review-1

(1 comment)

No objections to the patch, just a dumb question...

https://review.coreboot.org/c/coreboot/+/38290/3/util/xcompile/xcompile 
File util/xcompile/xcompile:

https://review.coreboot.org/c/coreboot/+/38290/3/util/xcompile/xcompile@296 
PS3, Line 296: NM_${TARCH}:=${GCCPREFIX}gcc-nm
Come to think of it, should these be guarded with something to only use these wrappers if GCC is used?

[Patrick Georgi (Code Review)]

Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38290 )

Change subject: xcompile: Use GCC wrappers for ar, nm
......................................................................

Patch Set 3:

(1 comment)

https://review.coreboot.org/c/coreboot/+/38290/3/util/xcompile/xcompile 
File util/xcompile/xcompile:

https://review.coreboot.org/c/coreboot/+/38290/3/util/xcompile/xcompile@296 
PS3, Line 296: NM_${TARCH}:=${GCCPREFIX}gcc-nm
> Come to think of it, should these be guarded with something to only use these wrappers if GCC is use […]

Our support for non-gcc compilers is _really_ limited so I wouldn't care too much about it right now. clang probably needs its own special variants of these tools.

[Jacob Garber (Code Review)]

Hello HAOUAS Elyes, Julius Werner, David Hendricks, build bot (Jenkins), Patrick Georgi, Martin Roth,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/38290

to look at the new patch set (#4).

Change subject: xcompile: Use GCC wrappers for ar, nm
......................................................................

xcompile: Use GCC wrappers for ar, nm

When compiling with GCC, use the special wrappers around ar and nm that
provide the path to the plugin they need to understand LTO object files.
These wrappers forward all other functionality to the underlying
programs, so they should otherwise be equivalent.

Change-Id: Ibdae4faabf67bf6a4bb8c38970f6189646ee74b3
Signed-off-by: Jacob Garber 
---
M util/xcompile/xcompile
1 file changed, 7 insertions(+), 2 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/90/38290/4

[Jacob Garber (Code Review)]

Jacob Garber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38290 )

Change subject: xcompile: Use GCC wrappers for ar, nm
......................................................................

Patch Set 4:

(1 comment)

https://review.coreboot.org/c/coreboot/+/38290/3/util/xcompile/xcompile 
File util/xcompile/xcompile:

https://review.coreboot.org/c/coreboot/+/38290/3/util/xcompile/xcompile@296 
PS3, Line 296: NM_${TARCH}:=${GCCPREFIX}gcc-nm
> Our support for non-gcc compilers is _really_ limited so I wouldn't care too much about it right now […]

I've wrapped these in an if statement so they're only used with GCC. I think Clang can work with regular binutils, but it would certainly need its own llvm-* versions if we ever wanted to use LTO as well.

[Paul Menzel (Code Review)]

Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38290 )

Change subject: xcompile: Use GCC wrappers for ar, nm
......................................................................

Patch Set 5: Code-Review+1

[Patrick Georgi (Code Review)]

Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38290 )

Change subject: xcompile: Use GCC wrappers for ar, nm
......................................................................

Patch Set 6: Code-Review+2

[Patrick Georgi (Code Review)]

Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/38290 )

Change subject: xcompile: Use GCC wrappers for ar, nm
......................................................................

xcompile: Use GCC wrappers for ar, nm

When compiling with GCC, use the special wrappers around ar and nm that
provide the path to the plugin they need to understand LTO object files.
These wrappers forward all other functionality to the underlying
programs, so they should otherwise be equivalent.

Change-Id: Ibdae4faabf67bf6a4bb8c38970f6189646ee74b3
Signed-off-by: Jacob Garber 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/38290
Reviewed-by: Patrick Georgi 
Reviewed-by: Paul Menzel 
Tested-by: build bot (Jenkins) 
---
M util/xcompile/xcompile
1 file changed, 7 insertions(+), 2 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Patrick Georgi: Looks good to me, approved
  Paul Menzel: Looks good to me, but someone else must approve

diff --git a/util/xcompile/xcompile b/util/xcompile/xcompile
index a116407..3203d71 100755
--- a/util/xcompile/xcompile
+++ b/util/xcompile/xcompile
@@ -293,12 +293,17 @@
    fi # if [ "${TARCH}" = "arm64" ]...

cat <<EOF
-NM_${TARCH}:=${GCCPREFIX}nm
+ifeq ($(CONFIG_COMPILER_GCC)$(CONFIG_LP_COMPILER_GCC),y)
+	NM_${TARCH}:=${GCCPREFIX}gcc-nm
+	AR_${TARCH}:=${GCCPREFIX}gcc-ar
+else
+	NM_${TARCH}:=${GCCPREFIX}nm
+	AR_${TARCH}:=${GCCPREFIX}ar
+endif
 OBJCOPY_${TARCH}:=${GCCPREFIX}objcopy
 OBJDUMP_${TARCH}:=${GCCPREFIX}objdump
 READELF_${TARCH}:=${GCCPREFIX}readelf
 STRIP_${TARCH}:=${GCCPREFIX}strip
-AR_${TARCH}:=${GCCPREFIX}ar
 GNATBIND_${TARCH}:=${GCCPREFIX}gnatbind
 CROSS_COMPILE_${TARCH}:=${GCCPREFIX}

[9elements QA (Code Review)]

9elements QA has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38290 )

Change subject: xcompile: Use GCC wrappers for ar, nm
......................................................................

Patch Set 7:

Automatic boot test returned (PASS/FAIL/TOTAL): 3/0/3
Emulation targets:
EMULATION_QEMU_X86_Q35 using payload TianoCore : SUCCESS : https://lava.9esec.io/r/803
EMULATION_QEMU_X86_Q35 using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/802
EMULATION_QEMU_X86_I440FX using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/801

Please note: This test is under development and might not be accurate at all!