[PATCH] D65634: [RISCV] Default to ilp32d/lp64d in RISC-V Linux

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL371494: [RISCV] Default to ilp32d/lp64d in RISC-V Linux 
(authored by rogfer01, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D65634?vs=213812=219487#toc

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65634/new/

https://reviews.llvm.org/D65634

Files:
  cfe/trunk/lib/Driver/ToolChains/Arch/RISCV.cpp
  cfe/trunk/test/Driver/riscv32-toolchain.c
  cfe/trunk/test/Driver/riscv64-toolchain.c
  cfe/trunk/test/Preprocessor/riscv-target-features.c


Index: cfe/trunk/test/Driver/riscv32-toolchain.c
===
--- cfe/trunk/test/Driver/riscv32-toolchain.c
+++ cfe/trunk/test/Driver/riscv32-toolchain.c
@@ -68,7 +68,7 @@
 // CXX-RV32-BAREMETAL-NOSYSROOT-ILP32: 
"{{.*}}/Inputs/basic_riscv32_tree/lib/gcc/riscv32-unknown-elf/8.0.1{{/|}}crtend.o"
 
 // RUN: %clang %s -### -no-canonical-prefixes -fuse-ld=ld \
-// RUN:   -target riscv32-unknown-linux-gnu \
+// RUN:   -target riscv32-unknown-linux-gnu -mabi=ilp32 \
 // RUN:   --gcc-toolchain=%S/Inputs/multilib_riscv_linux_sdk \
 // RUN:   --sysroot=%S/Inputs/multilib_riscv_linux_sdk/sysroot 2>&1 \
 // RUN:   | FileCheck -check-prefix=C-RV32-LINUX-MULTI-ILP32 %s
@@ -84,7 +84,7 @@
 // C-RV32-LINUX-MULTI-ILP32: 
"-L{{.*}}/Inputs/multilib_riscv_linux_sdk/sysroot/usr/lib32/ilp32"
 
 // RUN: %clang %s -### -no-canonical-prefixes -fuse-ld=ld \
-// RUN:   -target riscv32-unknown-linux-gnu -march=rv32imafd -mabi=ilp32d \
+// RUN:   -target riscv32-unknown-linux-gnu -march=rv32imafd \
 // RUN:   --gcc-toolchain=%S/Inputs/multilib_riscv_linux_sdk \
 // RUN:   --sysroot=%S/Inputs/multilib_riscv_linux_sdk/sysroot 2>&1 \
 // RUN:   | FileCheck -check-prefix=C-RV32-LINUX-MULTI-ILP32D %s
Index: cfe/trunk/test/Driver/riscv64-toolchain.c
===
--- cfe/trunk/test/Driver/riscv64-toolchain.c
+++ cfe/trunk/test/Driver/riscv64-toolchain.c
@@ -68,7 +68,7 @@
 // CXX-RV64-BAREMETAL-NOSYSROOT-LP64: 
"{{.*}}/Inputs/basic_riscv64_tree/lib/gcc/riscv64-unknown-elf/8.0.1{{/|}}crtend.o"
 
 // RUN: %clang %s -### -no-canonical-prefixes -fuse-ld=ld \
-// RUN:   -target riscv64-unknown-linux-gnu \
+// RUN:   -target riscv64-unknown-linux-gnu -mabi=lp64 \
 // RUN:   --gcc-toolchain=%S/Inputs/multilib_riscv_linux_sdk \
 // RUN:   --sysroot=%S/Inputs/multilib_riscv_linux_sdk/sysroot 2>&1 \
 // RUN:   | FileCheck -check-prefix=C-RV64-LINUX-MULTI-LP64 %s
@@ -84,7 +84,7 @@
 // C-RV64-LINUX-MULTI-LP64: 
"-L{{.*}}/Inputs/multilib_riscv_linux_sdk/sysroot/usr/lib64/lp64"
 
 // RUN: %clang %s -### -no-canonical-prefixes -fuse-ld=ld \
-// RUN:   -target riscv64-unknown-linux-gnu -march=rv64imafd -mabi=lp64d \
+// RUN:   -target riscv64-unknown-linux-gnu -march=rv64imafd \
 // RUN:   --gcc-toolchain=%S/Inputs/multilib_riscv_linux_sdk \
 // RUN:   --sysroot=%S/Inputs/multilib_riscv_linux_sdk/sysroot 2>&1 \
 // RUN:   | FileCheck -check-prefix=C-RV64-LINUX-MULTI-LP64D %s
Index: cfe/trunk/test/Preprocessor/riscv-target-features.c
===
--- cfe/trunk/test/Preprocessor/riscv-target-features.c
+++ cfe/trunk/test/Preprocessor/riscv-target-features.c
@@ -48,9 +48,9 @@
 // RUN: -o - | FileCheck --check-prefix=CHECK-C-EXT %s
 // CHECK-C-EXT: __riscv_compressed 1
 
-// RUN: %clang -target riscv32-unknown-linux-gnu -march=rv32ifd -x c -E -dM %s 
\
+// RUN: %clang -target riscv32-unknown-linux-gnu -march=rv32ifd -mabi=ilp32 -x 
c -E -dM %s \
 // RUN: -o - | FileCheck --check-prefix=CHECK-SOFT %s
-// RUN: %clang -target riscv64-unknown-linux-gnu -march=rv64ifd -x c -E -dM %s 
\
+// RUN: %clang -target riscv64-unknown-linux-gnu -march=rv64ifd -mabi=lp64 -x 
c -E -dM %s \
 // RUN: -o - | FileCheck --check-prefix=CHECK-SOFT %s
 // CHECK-SOFT: __riscv_float_abi_soft 1
 // CHECK-SOFT-NOT: __riscv_float_abi_single
@@ -64,9 +64,9 @@
 // CHECK-SINGLE-NOT: __riscv_float_abi_soft
 // CHECK-SINGLE-NOT: __riscv_float_abi_double
 
-// RUN: %clang -target riscv32-unknown-linux-gnu -march=rv32ifd -mabi=ilp32d 
-x c -E -dM %s \
+// RUN: %clang -target riscv32-unknown-linux-gnu -march=rv32ifd -x c -E -dM %s 
\
 // RUN: -o - | FileCheck --check-prefix=CHECK-DOUBLE %s
-// RUN: %clang -target riscv64-unknown-linux-gnu -march=rv64ifd -mabi=lp64d -x 
c -E -dM %s \
+// RUN: %clang -target riscv64-unknown-linux-gnu -march=rv64ifd -x c -E -dM %s 
\
 // RUN: -o - | FileCheck --check-prefix=CHECK-DOUBLE %s
 // CHECK-DOUBLE: __riscv_float_abi_double 1
 // CHECK-DOUBLE-NOT: __riscv_float_abi_soft
Index: cfe/trunk/lib/Driver/ToolChains/Arch/RISCV.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/Arch/RISCV.cpp
+++ cfe/trunk/lib/Driver/ToolChains/Arch/RISCV.cpp
@@ -389,7 +389,9 @@
   if (const Arg *A = 

[PATCH] D65634: [RISCV] Default to ilp32d/lp64d in RISC-V Linux

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

rogfer01 added a comment.

Thanks for the review @lenary @luismarques

We can indeed look at what defaults we want for baremetal in a later change.

I plan to commit this shortly.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65634/new/

https://reviews.llvm.org/D65634



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D65634: [RISCV] Default to ilp32d/lp64d in RISC-V Linux

[Sam Elliott via Phabricator via cfe-commits]

lenary accepted this revision.
lenary added a comment.
This revision is now accepted and ready to land.
Herald added a subscriber: pzheng.

I think my feeling is that this patch can land and we can change the default 
abi for baremetal targets in a follow-up patch.




Comment at: clang/lib/Driver/ToolChains/Arch/RISCV.cpp:386
+  else
+return Triple.getArch() == llvm::Triple::riscv32 ? "ilp32" : "lp64";
 }

luismarques wrote:
> luismarques wrote:
> > When I compile a bare metal GNU toolchain (using 
> > , reports GCC 8.3.0) I seem 
> > to get lp64d by default. Should we not match that behaviour?
> > 
> > ```
> > 
> > $ cat test.c
> > float foo() { return 42.0; }
> > $ riscv64-unknown-elf-gcc -O2 -S test.c
> > $ cat test.s
> > (...)
> > foo:
> > lui a5,%hi(.LC0)
> > flw fa0,%lo(.LC0)(a5)
> > (...)
> > ```
> To clarify, that's a toolchain configured with `--enable-multilib`.
I'm confused by this @luismarques 

If I do `riscv64-unknown-elf-gcc -c test.c` followed by 
`riscv64-unknown-elf-objdump -f test.o`, the flags displayed are 0x0011, 
which does not include `ELF::EF_RISCV_FLOAT_ABI_DOUBLE` (0x4), and so suggests 
gcc is using `-mabi=lp64` on baremetal elf targets. 

That said, `riscv64-unknown-elf-gcc -### -c test.c` shows it's invoking all 
subtools with `-march=rv64imafdc` and `-mabi-lp64d`. This is still with 
`--enable-multilib`, using `riscv64-unknown-elf-gcc (GCC) 8.3.0` and `GNU 
objdump (GNU Binutils) 2.32`.

I tried to look at where a default was being set in the gcc repo, and the only 
thing I can see is that the `rv64imafdc/lp64d` is the last combination to be 
generated in the multilib configuration, so they may not have explicitly chosen 
it as a default. I'm not sure. 


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65634/new/

https://reviews.llvm.org/D65634



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D65634: [RISCV] Default to ilp32d/lp64d in RISC-V Linux

[Luís Marques via Phabricator via cfe-commits]

luismarques added inline comments.



Comment at: clang/lib/Driver/ToolChains/Arch/RISCV.cpp:386
+  else
+return Triple.getArch() == llvm::Triple::riscv32 ? "ilp32" : "lp64";
 }

luismarques wrote:
> When I compile a bare metal GNU toolchain (using 
> , reports GCC 8.3.0) I seem to 
> get lp64d by default. Should we not match that behaviour?
> 
> ```
> 
> $ cat test.c
> float foo() { return 42.0; }
> $ riscv64-unknown-elf-gcc -O2 -S test.c
> $ cat test.s
> (...)
> foo:
> lui a5,%hi(.LC0)
> flw fa0,%lo(.LC0)(a5)
> (...)
> ```
To clarify, that's a toolchain configured with `--enable-multilib`.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65634/new/

https://reviews.llvm.org/D65634



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D65634: [RISCV] Default to ilp32d/lp64d in RISC-V Linux

[Luís Marques via Phabricator via cfe-commits]

luismarques added inline comments.



Comment at: clang/lib/Driver/ToolChains/Arch/RISCV.cpp:386
+  else
+return Triple.getArch() == llvm::Triple::riscv32 ? "ilp32" : "lp64";
 }

When I compile a bare metal GNU toolchain (using 
, reports GCC 8.3.0) I seem to 
get lp64d by default. Should we not match that behaviour?

```

$ cat test.c
float foo() { return 42.0; }
$ riscv64-unknown-elf-gcc -O2 -S test.c
$ cat test.s
(...)
foo:
lui a5,%hi(.LC0)
flw fa0,%lo(.LC0)(a5)
(...)
```


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65634/new/

https://reviews.llvm.org/D65634



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D65634: [RISCV] Default to ilp32d/lp64d in RISC-V Linux

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

rogfer01 added a comment.

Thanks for the clarification @asb. I've posted D66003 
 (depending on D66002 
) for that.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65634/new/

https://reviews.llvm.org/D65634



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D65634: [RISCV] Default to ilp32d/lp64d in RISC-V Linux

[Alex Bradbury via Phabricator via cfe-commits]

asb added a comment.

In D65634#1618443 , @rogfer01 wrote:

> Thanks @asb @lenary for the review!
>
> I understand that, after this change, we will also want to make 
> `-march=rv{32,64}gc` the default in Linux as well. Otherwise there will be an 
> ABI mismatch with the default `-march=rv{32.64}i` in a default invocation.
>
> Does this make sense?


Yes, that makes sense to me.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65634/new/

https://reviews.llvm.org/D65634



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D65634: [RISCV] Default to ilp32d/lp64d in RISC-V Linux

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

rogfer01 updated this revision to Diff 213812.
rogfer01 retitled this revision from "[RISCV] Default to lp64d in 64-bit RISC-V 
Linux" to "[RISCV] Default to ilp32d/lp64d in RISC-V Linux".
rogfer01 edited the summary of this revision.
rogfer01 added a comment.

ChangeLog:

- Make `ilp32d` also the default in 32-bit RISC-V Linux
- Do not use nested conditional expressions


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65634/new/

https://reviews.llvm.org/D65634

Files:
  clang/lib/Driver/ToolChains/Arch/RISCV.cpp
  clang/test/Driver/riscv32-toolchain.c
  clang/test/Driver/riscv64-toolchain.c
  clang/test/Preprocessor/riscv-target-features.c


Index: clang/test/Preprocessor/riscv-target-features.c
===
--- clang/test/Preprocessor/riscv-target-features.c
+++ clang/test/Preprocessor/riscv-target-features.c
@@ -48,9 +48,9 @@
 // RUN: -o - | FileCheck --check-prefix=CHECK-C-EXT %s
 // CHECK-C-EXT: __riscv_compressed 1
 
-// RUN: %clang -target riscv32-unknown-linux-gnu -march=rv32ifd -x c -E -dM %s 
\
+// RUN: %clang -target riscv32-unknown-linux-gnu -march=rv32ifd -mabi=ilp32 -x 
c -E -dM %s \
 // RUN: -o - | FileCheck --check-prefix=CHECK-SOFT %s
-// RUN: %clang -target riscv64-unknown-linux-gnu -march=rv64ifd -x c -E -dM %s 
\
+// RUN: %clang -target riscv64-unknown-linux-gnu -march=rv64ifd -mabi=lp64 -x 
c -E -dM %s \
 // RUN: -o - | FileCheck --check-prefix=CHECK-SOFT %s
 // CHECK-SOFT: __riscv_float_abi_soft 1
 // CHECK-SOFT-NOT: __riscv_float_abi_single
@@ -64,9 +64,9 @@
 // CHECK-SINGLE-NOT: __riscv_float_abi_soft
 // CHECK-SINGLE-NOT: __riscv_float_abi_double
 
-// RUN: %clang -target riscv32-unknown-linux-gnu -march=rv32ifd -mabi=ilp32d 
-x c -E -dM %s \
+// RUN: %clang -target riscv32-unknown-linux-gnu -march=rv32ifd -x c -E -dM %s 
\
 // RUN: -o - | FileCheck --check-prefix=CHECK-DOUBLE %s
-// RUN: %clang -target riscv64-unknown-linux-gnu -march=rv64ifd -mabi=lp64d -x 
c -E -dM %s \
+// RUN: %clang -target riscv64-unknown-linux-gnu -march=rv64ifd -x c -E -dM %s 
\
 // RUN: -o - | FileCheck --check-prefix=CHECK-DOUBLE %s
 // CHECK-DOUBLE: __riscv_float_abi_double 1
 // CHECK-DOUBLE-NOT: __riscv_float_abi_soft
Index: clang/test/Driver/riscv64-toolchain.c
===
--- clang/test/Driver/riscv64-toolchain.c
+++ clang/test/Driver/riscv64-toolchain.c
@@ -68,7 +68,7 @@
 // CXX-RV64-BAREMETAL-NOSYSROOT-LP64: 
"{{.*}}/Inputs/basic_riscv64_tree/lib/gcc/riscv64-unknown-elf/8.0.1{{/|}}crtend.o"
 
 // RUN: %clang %s -### -no-canonical-prefixes -fuse-ld=ld \
-// RUN:   -target riscv64-unknown-linux-gnu \
+// RUN:   -target riscv64-unknown-linux-gnu -mabi=lp64 \
 // RUN:   --gcc-toolchain=%S/Inputs/multilib_riscv_linux_sdk \
 // RUN:   --sysroot=%S/Inputs/multilib_riscv_linux_sdk/sysroot 2>&1 \
 // RUN:   | FileCheck -check-prefix=C-RV64-LINUX-MULTI-LP64 %s
@@ -84,7 +84,7 @@
 // C-RV64-LINUX-MULTI-LP64: 
"-L{{.*}}/Inputs/multilib_riscv_linux_sdk/sysroot/usr/lib64/lp64"
 
 // RUN: %clang %s -### -no-canonical-prefixes -fuse-ld=ld \
-// RUN:   -target riscv64-unknown-linux-gnu -march=rv64imafd -mabi=lp64d \
+// RUN:   -target riscv64-unknown-linux-gnu -march=rv64imafd \
 // RUN:   --gcc-toolchain=%S/Inputs/multilib_riscv_linux_sdk \
 // RUN:   --sysroot=%S/Inputs/multilib_riscv_linux_sdk/sysroot 2>&1 \
 // RUN:   | FileCheck -check-prefix=C-RV64-LINUX-MULTI-LP64D %s
Index: clang/test/Driver/riscv32-toolchain.c
===
--- clang/test/Driver/riscv32-toolchain.c
+++ clang/test/Driver/riscv32-toolchain.c
@@ -68,7 +68,7 @@
 // CXX-RV32-BAREMETAL-NOSYSROOT-ILP32: 
"{{.*}}/Inputs/basic_riscv32_tree/lib/gcc/riscv32-unknown-elf/8.0.1{{/|}}crtend.o"
 
 // RUN: %clang %s -### -no-canonical-prefixes -fuse-ld=ld \
-// RUN:   -target riscv32-unknown-linux-gnu \
+// RUN:   -target riscv32-unknown-linux-gnu -mabi=ilp32 \
 // RUN:   --gcc-toolchain=%S/Inputs/multilib_riscv_linux_sdk \
 // RUN:   --sysroot=%S/Inputs/multilib_riscv_linux_sdk/sysroot 2>&1 \
 // RUN:   | FileCheck -check-prefix=C-RV32-LINUX-MULTI-ILP32 %s
@@ -84,7 +84,7 @@
 // C-RV32-LINUX-MULTI-ILP32: 
"-L{{.*}}/Inputs/multilib_riscv_linux_sdk/sysroot/usr/lib32/ilp32"
 
 // RUN: %clang %s -### -no-canonical-prefixes -fuse-ld=ld \
-// RUN:   -target riscv32-unknown-linux-gnu -march=rv32imafd -mabi=ilp32d \
+// RUN:   -target riscv32-unknown-linux-gnu -march=rv32imafd \
 // RUN:   --gcc-toolchain=%S/Inputs/multilib_riscv_linux_sdk \
 // RUN:   --sysroot=%S/Inputs/multilib_riscv_linux_sdk/sysroot 2>&1 \
 // RUN:   | FileCheck -check-prefix=C-RV32-LINUX-MULTI-ILP32D %s
Index: clang/lib/Driver/ToolChains/Arch/RISCV.cpp
===
--- clang/lib/Driver/ToolChains/Arch/RISCV.cpp
+++ clang/lib/Driver/ToolChains/Arch/RISCV.cpp
@@ -379,7 +379,9 @@
   if (const Arg *A = Args.getLastArg(options::OPT_mabi_EQ))
 return 

[PATCH] D65634: [RISCV] Default to ilp32d/lp64d in RISC-V Linux

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

rogfer01 added a comment.

Thanks @asb @lenary for the review!

I understand that, after this change, we will also want to make 
`-march=rv{32,64}gc` the default in Linux as well. Otherwise there will be an 
ABI mismatch with the default `-march=rv{32.64}i` in a default invocation.

Does this make sense?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65634/new/

https://reviews.llvm.org/D65634



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits