| Message ID | 20211202122929.685756-2-anup.patel@wdc.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Misc compile error/warning fixes | expand |
On Dec 2, 2021, at 8:29 PM, anup patel anup.patel@wdc.com wrote: > The riscv target of CLANG does not support -m(no-)save-restore option > so we get compile warnings. This patch fixes compile warning by using > -m(no-)save-restore option only for GCC. > > Signed-off-by: Anup Patel <anup.patel@wdc.com> Looks good to me. Reviewed-by: Dong Du <Dd_nirvana@sjtu.edu.cn> > --- > Makefile | 12 ++++++++---- > 1 file changed, 8 insertions(+), 4 deletions(-) > > diff --git a/Makefile b/Makefile > index 8623c1c..1e35dc0 100644 > --- a/Makefile > +++ b/Makefile > @@ -275,8 +275,10 @@ GENFLAGS += $(platform-genflags-y) > GENFLAGS += $(firmware-genflags-y) > > CFLAGS = -g -Wall -Werror -ffreestanding -nostdlib -fno-stack-protector > -fno-strict-aliasing -O2 > -CFLAGS += -fno-omit-frame-pointer -fno-optimize-sibling-calls > -CFLAGS += -mno-save-restore -mstrict-align > +CFLAGS += -fno-omit-frame-pointer -fno-optimize-sibling-calls -mstrict-align > +ifneq ($(CC_IS_CLANG),y) > +CFLAGS += -mno-save-restore > +endif > CFLAGS += -mabi=$(PLATFORM_RISCV_ABI) -march=$(PLATFORM_RISCV_ISA) > CFLAGS += -mcmodel=$(PLATFORM_RISCV_CODE_MODEL) > CFLAGS += $(RELAX_FLAG) > @@ -290,8 +292,10 @@ CPPFLAGS += $(platform-cppflags-y) > CPPFLAGS += $(firmware-cppflags-y) > > ASFLAGS = -g -Wall -nostdlib > -ASFLAGS += -fno-omit-frame-pointer -fno-optimize-sibling-calls > -ASFLAGS += -mno-save-restore -mstrict-align > +ASFLAGS += -fno-omit-frame-pointer -fno-optimize-sibling-calls -mstrict-align > +ifneq ($(CC_IS_CLANG),y) > +ASFLAGS += -mno-save-restore > +endif > ASFLAGS += -mabi=$(PLATFORM_RISCV_ABI) -march=$(PLATFORM_RISCV_ISA) > ASFLAGS += -mcmodel=$(PLATFORM_RISCV_CODE_MODEL) > ASFLAGS += $(RELAX_FLAG) > -- > 2.25.1 > > > -- > opensbi mailing list > opensbi@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/opensbi
在 2021-12-02星期四的 17:59 +0530，Anup Patel写道： > The riscv target of CLANG does not support -m(no-)save-restore option > so we get compile warnings. This patch fixes compile warning by using > -m(no-)save-restore option only for GCC. > > Signed-off-by: Anup Patel <anup.patel@wdc.com> ➜ /tmp clang -target riscv64 -mno-save-restore -O2 -S test.c ➜ /tmp clang -target riscv64 -msave-restore -O2 -S test.c ➜ /tmp clang --version Debian clang version 11.0.1-2 Target: x86_64-pc-linux-gnu Thread model: posix InstalledDir: /usr/bin ➜ /tmp lsb_release -a No LSB modules are available. Distributor ID: Debian Description: Debian GNU/Linux 11 (bullseye) Release: 11 Codename: bullseye ➜ /tmp clang supports -m(no-)save-restore option on my computer Regards, Xiang W > --- > Makefile | 12 ++++++++---- > 1 file changed, 8 insertions(+), 4 deletions(-) > > diff --git a/Makefile b/Makefile > index 8623c1c..1e35dc0 100644 > --- a/Makefile > +++ b/Makefile > @@ -275,8 +275,10 @@ GENFLAGS += $(platform-genflags-y) > GENFLAGS += $(firmware-genflags-y) > > CFLAGS = -g -Wall -Werror -ffreestanding -nostdlib - > fno-stack-protector -fno-strict-aliasing -O2 > -CFLAGS += -fno-omit-frame-pointer -fno-optimize- > sibling-calls > -CFLAGS += -mno-save-restore -mstrict-align > +CFLAGS += -fno-omit-frame-pointer -fno-optimize- > sibling-calls -mstrict-align > +ifneq ($(CC_IS_CLANG),y) > +CFLAGS += -mno-save-restore > +endif > CFLAGS += -mabi=$(PLATFORM_RISCV_ABI) - > march=$(PLATFORM_RISCV_ISA) > CFLAGS += -mcmodel=$(PLATFORM_RISCV_CODE_MODEL) > CFLAGS += $(RELAX_FLAG) > @@ -290,8 +292,10 @@ CPPFLAGS += $(platform-cppflags-y) > CPPFLAGS += $(firmware-cppflags-y) > > ASFLAGS = -g -Wall -nostdlib > -ASFLAGS += -fno-omit-frame-pointer -fno- > optimize-sibling-calls > -ASFLAGS += -mno-save-restore -mstrict-align > +ASFLAGS += -fno-omit-frame-pointer -fno- > optimize-sibling-calls -mstrict-align > +ifneq ($(CC_IS_CLANG),y) > +ASFLAGS += -mno-save-restore > +endif > ASFLAGS += -mabi=$(PLATFORM_RISCV_ABI) - > march=$(PLATFORM_RISCV_ISA) > ASFLAGS += -mcmodel=$(PLATFORM_RISCV_CODE_MODEL) > ASFLAGS += $(RELAX_FLAG) > -- > 2.25.1 > >
On 2 Dec 2021, at 14:28, Xiang W <wxjstz@126.com> wrote: > > 在 2021-12-02星期四的 17:59 +0530，Anup Patel写道： >> The riscv target of CLANG does not support -m(no-)save-restore option >> so we get compile warnings. This patch fixes compile warning by using >> -m(no-)save-restore option only for GCC. >> >> Signed-off-by: Anup Patel <anup.patel@wdc.com> > > ➜ /tmp clang -target riscv64 -mno-save-restore -O2 -S test.c > ➜ /tmp clang -target riscv64 -msave-restore -O2 -S test.c > ➜ /tmp clang --version > Debian clang version 11.0.1-2 > Target: x86_64-pc-linux-gnu > Thread model: posix > InstalledDir: /usr/bin > ➜ /tmp lsb_release -a > No LSB modules are available. > Distributor ID: Debian > Description: Debian GNU/Linux 11 (bullseye) > Release: 11 > Codename: bullseye > ➜ /tmp > > clang supports -m(no-)save-restore option on my computer Yes, Clang 11 and above support -m(no-)save-restore. Clang 9 and 10 support the driver option but were buggy and would cause the backend to give several warnings even if you just gave -mno-save-restore (https://reviews.llvm.org/D64008, which did make it into Clang 9, was not a complete fix for that, see my comment there). Clang 8 and below didn’t support the option in the driver, but the RISCV backend only graduated from experimental status in Clang 9 so, whilst it was in relatively decent shape before then, such versions should probably be discouraged. In theory not passing the option is fine, there isn’t currently a way to configure Clang to default to -msave-restore other than patching it, though only skipping it for Clang 10 and below would be preferable. Either way the commit message needs updating. Or you could just say it’s not worth caring and leave the warnings for Clang 9 and 10; with time they’ll stop being relevant, and they’re stderr output, not real diagnostics, so -Werror doesn’t make them fatal. That’s probably the approach I’d take, but I don’t have to work with old versions of Clang on RISC-V. Jess
On Thu, Dec 2, 2021 at 7:58 PM Xiang W <wxjstz@126.com> wrote: > > 在 2021-12-02星期四的 17:59 +0530，Anup Patel写道： > > The riscv target of CLANG does not support -m(no-)save-restore option > > so we get compile warnings. This patch fixes compile warning by using > > -m(no-)save-restore option only for GCC. > > > > Signed-off-by: Anup Patel <anup.patel@wdc.com> > > ➜ /tmp clang -target riscv64 -mno-save-restore -O2 -S test.c > ➜ /tmp clang -target riscv64 -msave-restore -O2 -S test.c > ➜ /tmp clang --version > Debian clang version 11.0.1-2 > Target: x86_64-pc-linux-gnu > Thread model: posix > InstalledDir: /usr/bin > ➜ /tmp lsb_release -a > No LSB modules are available. > Distributor ID: Debian > Description: Debian GNU/Linux 11 (bullseye) > Release: 11 > Codename: bullseye > ➜ /tmp > > clang supports -m(no-)save-restore option on my computer I am using clang package available on Ubuntu-20.04 LTS which is still at clang-10. Regards, Anup > > Regards, > Xiang W > > --- > > Makefile | 12 ++++++++---- > > 1 file changed, 8 insertions(+), 4 deletions(-) > > > > diff --git a/Makefile b/Makefile > > index 8623c1c..1e35dc0 100644 > > --- a/Makefile > > +++ b/Makefile > > @@ -275,8 +275,10 @@ GENFLAGS += $(platform-genflags-y) > > GENFLAGS += $(firmware-genflags-y) > > > > CFLAGS = -g -Wall -Werror -ffreestanding -nostdlib - > > fno-stack-protector -fno-strict-aliasing -O2 > > -CFLAGS += -fno-omit-frame-pointer -fno-optimize- > > sibling-calls > > -CFLAGS += -mno-save-restore -mstrict-align > > +CFLAGS += -fno-omit-frame-pointer -fno-optimize- > > sibling-calls -mstrict-align > > +ifneq ($(CC_IS_CLANG),y) > > +CFLAGS += -mno-save-restore > > +endif > > CFLAGS += -mabi=$(PLATFORM_RISCV_ABI) - > > march=$(PLATFORM_RISCV_ISA) > > CFLAGS += -mcmodel=$(PLATFORM_RISCV_CODE_MODEL) > > CFLAGS += $(RELAX_FLAG) > > @@ -290,8 +292,10 @@ CPPFLAGS += $(platform-cppflags-y) > > CPPFLAGS += $(firmware-cppflags-y) > > > > ASFLAGS = -g -Wall -nostdlib > > -ASFLAGS += -fno-omit-frame-pointer -fno- > > optimize-sibling-calls > > -ASFLAGS += -mno-save-restore -mstrict-align > > +ASFLAGS += -fno-omit-frame-pointer -fno- > > optimize-sibling-calls -mstrict-align > > +ifneq ($(CC_IS_CLANG),y) > > +ASFLAGS += -mno-save-restore > > +endif > > ASFLAGS += -mabi=$(PLATFORM_RISCV_ABI) - > > march=$(PLATFORM_RISCV_ISA) > > ASFLAGS += -mcmodel=$(PLATFORM_RISCV_CODE_MODEL) > > ASFLAGS += $(RELAX_FLAG) > > -- > > 2.25.1 > > > > > >
On Thu, Dec 2, 2021 at 8:43 PM Jessica Clarke <jrtc27@jrtc27.com> wrote: > > On 2 Dec 2021, at 14:28, Xiang W <wxjstz@126.com> wrote: > > > > 在 2021-12-02星期四的 17:59 +0530，Anup Patel写道： > >> The riscv target of CLANG does not support -m(no-)save-restore option > >> so we get compile warnings. This patch fixes compile warning by using > >> -m(no-)save-restore option only for GCC. > >> > >> Signed-off-by: Anup Patel <anup.patel@wdc.com> > > > > ➜ /tmp clang -target riscv64 -mno-save-restore -O2 -S test.c > > ➜ /tmp clang -target riscv64 -msave-restore -O2 -S test.c > > ➜ /tmp clang --version > > Debian clang version 11.0.1-2 > > Target: x86_64-pc-linux-gnu > > Thread model: posix > > InstalledDir: /usr/bin > > ➜ /tmp lsb_release -a > > No LSB modules are available. > > Distributor ID: Debian > > Description: Debian GNU/Linux 11 (bullseye) > > Release: 11 > > Codename: bullseye > > ➜ /tmp > > > > clang supports -m(no-)save-restore option on my computer > > Yes, Clang 11 and above support -m(no-)save-restore. Clang 9 and 10 > support the driver option but were buggy and would cause the backend to > give several warnings even if you just gave -mno-save-restore > (https://reviews.llvm.org/D64008, which did make it into Clang 9, was > not a complete fix for that, see my comment there). Clang 8 and below > didn’t support the option in the driver, but the RISCV backend only > graduated from experimental status in Clang 9 so, whilst it was in > relatively decent shape before then, such versions should probably be > discouraged. > > In theory not passing the option is fine, there isn’t currently a way > to configure Clang to default to -msave-restore other than patching it, > though only skipping it for Clang 10 and below would be preferable. > Either way the commit message needs updating. Okay, I will update the commit message and also add a comment in Makefile. > > Or you could just say it’s not worth caring and leave the warnings for > Clang 9 and 10; with time they’ll stop being relevant, and they’re > stderr output, not real diagnostics, so -Werror doesn’t make them fatal. > That’s probably the approach I’d take, but I don’t have to work with > old versions of Clang on RISC-V. I am inclined towards suppressing this warning for now because apart from this warning OpenSBI works fine with clang-10. Once all major distros have moved to clang-11 (or higher), we can update the Makefile (maybe after 6-12 months). Regards, Anup > > Jess >
在 2021-12-02星期四的 15:12 +0000，Jessica Clarke写道： > On 2 Dec 2021, at 14:28, Xiang W <wxjstz@126.com> wrote: > > > > 在 2021-12-02星期四的 17:59 +0530，Anup Patel写道： > > > The riscv target of CLANG does not support -m(no-)save-restore > > > option > > > so we get compile warnings. This patch fixes compile warning by > > > using > > > -m(no-)save-restore option only for GCC. > > > > > > Signed-off-by: Anup Patel <anup.patel@wdc.com> > > > > ➜ /tmp clang -target riscv64 -mno-save-restore -O2 -S test.c > > ➜ /tmp clang -target riscv64 -msave-restore -O2 -S test.c > > ➜ /tmp clang --version > > Debian clang version 11.0.1-2 > > Target: x86_64-pc-linux-gnu > > Thread model: posix > > InstalledDir: /usr/bin > > ➜ /tmp lsb_release -a > > No LSB modules are available. > > Distributor ID: Debian > > Description: Debian GNU/Linux 11 (bullseye) > > Release: 11 > > Codename: bullseye > > ➜ /tmp > > > > clang supports -m(no-)save-restore option on my computer > > Yes, Clang 11 and above support -m(no-)save-restore. Clang 9 and 10 > support the driver option but were buggy and would cause the backend > to > give several warnings even if you just gave -mno-save-restore > (https://reviews.llvm.org/D64008, which did make it into Clang 9, was > not a complete fix for that, see my comment there). Clang 8 and below > didn’t support the option in the driver, but the RISCV backend only > graduated from experimental status in Clang 9 so, whilst it was in > relatively decent shape before then, such versions should probably be > discouraged. > > In theory not passing the option is fine, there isn’t currently a way > to configure Clang to default to -msave-restore other than patching > it, > though only skipping it for Clang 10 and below would be preferable. > Either way the commit message needs updating. > > Or you could just say it’s not worth caring and leave the warnings > for > Clang 9 and 10; with time they’ll stop being relevant, and they’re > stderr output, not real diagnostics, so -Werror doesn’t make them > fatal. > That’s probably the approach I’d take, but I don’t have to work with > old versions of Clang on RISC-V. > > Jess Thanks for the reply. Regards, Xiang W
diff --git a/Makefile b/Makefile index 8623c1c..1e35dc0 100644 --- a/Makefile +++ b/Makefile @@ -275,8 +275,10 @@ GENFLAGS += $(platform-genflags-y) GENFLAGS += $(firmware-genflags-y) CFLAGS = -g -Wall -Werror -ffreestanding -nostdlib -fno-stack-protector -fno-strict-aliasing -O2 -CFLAGS += -fno-omit-frame-pointer -fno-optimize-sibling-calls -CFLAGS += -mno-save-restore -mstrict-align +CFLAGS += -fno-omit-frame-pointer -fno-optimize-sibling-calls -mstrict-align +ifneq ($(CC_IS_CLANG),y) +CFLAGS += -mno-save-restore +endif CFLAGS += -mabi=$(PLATFORM_RISCV_ABI) -march=$(PLATFORM_RISCV_ISA) CFLAGS += -mcmodel=$(PLATFORM_RISCV_CODE_MODEL) CFLAGS += $(RELAX_FLAG) @@ -290,8 +292,10 @@ CPPFLAGS += $(platform-cppflags-y) CPPFLAGS += $(firmware-cppflags-y) ASFLAGS = -g -Wall -nostdlib -ASFLAGS += -fno-omit-frame-pointer -fno-optimize-sibling-calls -ASFLAGS += -mno-save-restore -mstrict-align +ASFLAGS += -fno-omit-frame-pointer -fno-optimize-sibling-calls -mstrict-align +ifneq ($(CC_IS_CLANG),y) +ASFLAGS += -mno-save-restore +endif ASFLAGS += -mabi=$(PLATFORM_RISCV_ABI) -march=$(PLATFORM_RISCV_ISA) ASFLAGS += -mcmodel=$(PLATFORM_RISCV_CODE_MODEL) ASFLAGS += $(RELAX_FLAG)
The riscv target of CLANG does not support -m(no-)save-restore option so we get compile warnings. This patch fixes compile warning by using -m(no-)save-restore option only for GCC. Signed-off-by: Anup Patel <anup.patel@wdc.com> --- Makefile | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-)