[PATCH] Makefile: Use -mstack-alignment=4 instead of -mpreferred-stack-boundary=2 for clang
( depends on https://mail.coreboot.org/hyperkitty/list/seabios@seabios.org/thread/SWDV7MB... "[PATCH] Makefile: Refactor cc-option"
see linux/arch/x86/Makefile for a similar use case )
Signed-off-by: Fangrui Song maskray@google.com --- Makefile | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/Makefile b/Makefile index 7720db9..eae7ac0 100644 --- a/Makefile +++ b/Makefile @@ -59,7 +59,7 @@ CPPFLAGS = -P -MD -MT $@ COMMONCFLAGS := -I$(OUT) -Isrc -Os -MD -g \ -Wall -Wno-strict-aliasing -Wold-style-definition \ $(call cc-option,-Wtype-limits) \ - -m32 -march=i386 -mregparm=3 -mpreferred-stack-boundary=2 \ + -m32 -march=i386 -mregparm=3 \ -minline-all-stringops -fomit-frame-pointer \ -freg-struct-return -ffreestanding -fno-delete-null-pointer-checks \ -ffunction-sections -fdata-sections -fno-common -fno-merge-constants @@ -69,6 +69,7 @@ COMMONCFLAGS += $(call cc-option,-fno-stack-protector) COMMONCFLAGS += $(call cc-option,-fno-stack-protector-all) COMMONCFLAGS += $(call cc-option,-fstack-check=no) COMMONCFLAGS += $(call cc-option,-Wno-address-of-packed-member) +COMMONCFLAGS += $(call cc-option,-mpreferred-stack-boundary=2,-mstack-alignment=4) COMMA := ,
CFLAGS32FLAT := $(COMMONCFLAGS) -DMODE16=0 -DMODESEGMENT=0
Sorry, v1 does not work with GCC. gcc x86_64 rejects -mpreferred-stack-boundary=2, so we need to check -mpreferred-stack-boundary=4 instead
Signed-off-by: Fangrui Song maskray@google.com --- Makefile | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/Makefile b/Makefile index 7720db9..66b3eab 100644 --- a/Makefile +++ b/Makefile @@ -59,7 +59,7 @@ CPPFLAGS = -P -MD -MT $@ COMMONCFLAGS := -I$(OUT) -Isrc -Os -MD -g \ -Wall -Wno-strict-aliasing -Wold-style-definition \ $(call cc-option,-Wtype-limits) \ - -m32 -march=i386 -mregparm=3 -mpreferred-stack-boundary=2 \ + -m32 -march=i386 -mregparm=3 \ -minline-all-stringops -fomit-frame-pointer \ -freg-struct-return -ffreestanding -fno-delete-null-pointer-checks \ -ffunction-sections -fdata-sections -fno-common -fno-merge-constants @@ -69,6 +69,11 @@ COMMONCFLAGS += $(call cc-option,-fno-stack-protector) COMMONCFLAGS += $(call cc-option,-fno-stack-protector-all) COMMONCFLAGS += $(call cc-option,-fstack-check=no) COMMONCFLAGS += $(call cc-option,-Wno-address-of-packed-member) +ifneq ($(call cc-option,-mpreferred-stack-boundary=4),) +COMMONCFLAGS += -mpreferred-stack-boundary=2 +else +COMMONCFLAGS += -mstack-alignment=4 +endif COMMA := ,
CFLAGS32FLAT := $(COMMONCFLAGS) -DMODE16=0 -DMODESEGMENT=0
On Sun, Mar 15, 2020 at 10:52:56AM -0700, Fangrui Song wrote:
( depends on https://mail.coreboot.org/hyperkitty/list/seabios@seabios.org/thread/SWDV7MB... "[PATCH] Makefile: Refactor cc-option"
see linux/arch/x86/Makefile for a similar use case )
Better patch description please. You should also send the patches as series, that'll take care of the dependencies, and you can also describe the purpose of whole series, requirements and testing status in the cover letter.
Purpose seems to be to make seabios buildable with clang. A note about the required clang versions would be useful. Building with gcc must continue to work of course. Did you test that? Which gcc version?
cheers, Gerd
On Mon, Mar 16, 2020 at 1:05 AM Gerd Hoffmann kraxel@redhat.com wrote:
On Sun, Mar 15, 2020 at 10:52:56AM -0700, Fangrui Song wrote:
( depends on https://mail.coreboot.org/hyperkitty/list/seabios@seabios.org/thread/SWDV7MB... "[PATCH] Makefile: Refactor cc-option"
see linux/arch/x86/Makefile for a similar use case )
Better patch description please. You should also send the patches as series, that'll take care of the dependencies, and you can also describe the purpose of whole series, requirements and testing status in the cover letter.
Purpose seems to be to make seabios buildable with clang. A note about the required clang versions would be useful. Building with gcc must continue to work of course. Did you test that? Which gcc version?
Most patches don't have a dependency. They can be applied in arbitrary order. This patch depends on "[PATCH] Makefile: Refactor cc-option" just because that patch removes $(CC) and this patch deletes a line without $(CC).
This patch can be applied without "[PATCH] Makefile: Refactor cc-option" if ,$(CC) is added back.
I tested with GCC 9.2.1. Note that clang still does not build due to other reasons:
(1) probable misuse of constraint code 'Q': (2) clang cannot handle a typeof on GNU expression statement extension. I haven't investigated whether it is a clang bug.
Dear Fāng-ruì,
Am 16.03.20 um 15:30 schrieb Fāng-ruì Sòng:
On Mon, Mar 16, 2020 at 1:05 AM Gerd Hoffmann kraxel@redhat.com wrote:
On Sun, Mar 15, 2020 at 10:52:56AM -0700, Fangrui Song wrote:
( depends on https://mail.coreboot.org/hyperkitty/list/seabios@seabios.org/thread/SWDV7MB... "[PATCH] Makefile: Refactor cc-option"
see linux/arch/x86/Makefile for a similar use case )
Better patch description please. You should also send the patches as series, that'll take care of the dependencies, and you can also describe the purpose of whole series, requirements and testing status in the cover letter.
Purpose seems to be to make seabios buildable with clang. A note about the required clang versions would be useful. Building with gcc must continue to work of course. Did you test that? Which gcc version?
Most patches don't have a dependency. They can be applied in arbitrary order. This patch depends on "[PATCH] Makefile: Refactor cc-option" just because that patch removes $(CC) and this patch deletes a line without $(CC).
This patch can be applied without "[PATCH] Makefile: Refactor cc-option" if ,$(CC) is added back.
That is not the point. Please send at least patches depending on other as series. This way developers can use the known tools (no mail archive URLs), and you do not have to mention the dependency at all.
I tested with GCC 9.2.1. Note that clang still does not build due to other reasons:
(1) probable misuse of constraint code 'Q': (2) clang cannot handle a typeof on GNU expression statement extension. I haven't investigated whether it is a clang bug.
Please do not mix topics. It’s better to start a new thread with a descriptive subject line.
Kind regards,
Paul
On Mon, Mar 16, 2020 at 7:34 AM Paul Menzel pmenzel@molgen.mpg.de wrote:
Dear Fāng-ruì,
Am 16.03.20 um 15:30 schrieb Fāng-ruì Sòng:
On Mon, Mar 16, 2020 at 1:05 AM Gerd Hoffmann kraxel@redhat.com wrote:
On Sun, Mar 15, 2020 at 10:52:56AM -0700, Fangrui Song wrote:
( depends on https://mail.coreboot.org/hyperkitty/list/seabios@seabios.org/thread/SWDV7MB... "[PATCH] Makefile: Refactor cc-option"
see linux/arch/x86/Makefile for a similar use case )
Better patch description please. You should also send the patches as series, that'll take care of the dependencies, and you can also describe the purpose of whole series, requirements and testing status in the cover letter.
Purpose seems to be to make seabios buildable with clang. A note about the required clang versions would be useful. Building with gcc must continue to work of course. Did you test that? Which gcc version?
Most patches don't have a dependency. They can be applied in arbitrary order. This patch depends on "[PATCH] Makefile: Refactor cc-option" just because that patch removes $(CC) and this patch deletes a line without $(CC).
This patch can be applied without "[PATCH] Makefile: Refactor cc-option" if ,$(CC) is added back.
That is not the point. Please send at least patches depending on other as series. This way developers can use the known tools (no mail archive URLs), and you do not have to mention the dependency at all.
For this case, the dependency is really loose.
"[PATCH] Makefile: Use -mstack-alignment=4 instead of -mpreferred-stack-boundary=2 for clang" and "[PATCH] Makefile: Delete compiler driver option -nopie"
refer to patch lines with ,$(CC) removed so they depend on "[PATCH] Makefile: Refactor cc-option"
Do they still need to have a cover letter (0/3)? When I realize that -nopie should be deleted as well, it was a while after I sent out "[PATCH] Makefile: Use -mstack-alignment=4 instead of -mpreferred-stack-boundary=2 for clang"
I tested with GCC 9.2.1. Note that clang still does not build due to other reasons:
(1) probable misuse of constraint code 'Q': (2) clang cannot handle a typeof on GNU expression statement extension. I haven't investigated whether it is a clang bug.
Please do not mix topics. It’s better to start a new thread with a descriptive subject line.
Kind regards,
Paul
On Mon, Mar 16, 2020 at 07:30:30AM -0700, Fāng-ruì Sòng wrote:
On Mon, Mar 16, 2020 at 1:05 AM Gerd Hoffmann kraxel@redhat.com wrote:
On Sun, Mar 15, 2020 at 10:52:56AM -0700, Fangrui Song wrote:
( depends on https://mail.coreboot.org/hyperkitty/list/seabios@seabios.org/thread/SWDV7MB... "[PATCH] Makefile: Refactor cc-option"
see linux/arch/x86/Makefile for a similar use case )
Better patch description please. You should also send the patches as series, that'll take care of the dependencies, and you can also describe the purpose of whole series, requirements and testing status in the cover letter.
Purpose seems to be to make seabios buildable with clang. A note about the required clang versions would be useful. Building with gcc must continue to work of course. Did you test that? Which gcc version?
Most patches don't have a dependency. They can be applied in arbitrary order.
Well, nevertheless they all belong together as they are part of the effort to make seabios buildable with clang. So grouping them into a series and adding a cover letter with a high-level description and status still makes sense.
cheers, Gerd
-
Fangrui Song -
Fāng-ruì Sòng
-
Gerd Hoffmann -
Paul Menzel