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 
> > <https://github.com/riscv/riscv-gnu-toolchain>, 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 0x00000011, 
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... Sam Elliott via Phabricator via cfe-commits

Reply via email to