[PATCH] D60456: [RISCV] Hard float ABI support

[Alex Bradbury via Phabricator via cfe-commits]

asb marked an inline comment as done.
asb added inline comments.


================
Comment at: clang/lib/CodeGen/TargetInfo.cpp:9352
+        return false;
+      // As a quirk of the ABI, zero-width bitfields aren't ignored for fp+fp
+      // or int+fp structs, but are ignored for a struct with an fp field and
----------------
rogfer01 wrote:
> I found some mismatch in behaviour between gcc and g++ that we may want to address in the psABI first.
> 
> For instance, given the following struct (I'm using gcc 8.3.0)
> 
> ```lang=cpp
> // t.c
> struct A
> {
>   int :0;
>   double d;
>   int :0;
>   long x;
>   int :0;
> };
> 
> extern void bar(struct A);
> void foo(struct A a)
> {
>   a.d =- a.d;
>   a.x += 1;
>   return bar(a);
> }
> ```
> 
> we are emitting this
> 
> ```
> $ clang --target=riscv64 -march=rv64gc -mabi=lp64d -S -o-  t.c -O2  
> ...
> foo:                                    # @foo
> # %bb.0:                                # %entry
>         addi    a2, zero, -1
>         slli    a2, a2, 63
>         xor     a0, a0, a2
>         addi    a1, a1, 1
>         tail    bar
> ```
> 
> which matches with what g++ does (i.e in both cases `a0` is `a.d` and `a1` is `a.x`)
> 
> ```
> $ ./riscv64-unknown-linux-gnu-g++ -S -O2 -o- -x c test.cc
> ...
> foo:
> 	fmv.d.x	fa5,a0
> 	addi	sp,sp,-16
> 	fneg.d	fa5,fa5
> 	addi	a1,a1,1
> 	addi	sp,sp,16
> 	fmv.x.d	a0,fa5
> 	tail	bar
> ```
> 
> But I found a mismatch while using C++. Clang emits the same for C and C++ (modulo `.cfi` stuff)
> 
> ```
> $ clang --target=riscv64 -march=rv64gc -mabi=lp64d -S -o-  -x c++ t.c -O2  
> _Z3foo1A:                               # @_Z3foo1A
>         .cfi_startproc
> # %bb.0:                                # %entry
>         addi    a2, zero, -1
>         slli    a2, a2, 63
>         xor     a0, a0, a2
>         addi    a1, a1, 1
>         .cfi_def_cfa_offset 0
>         tail    _Z3bar1A
> ```
> 
> But g++ seems to ignore the zero-width bitfields: `fa0` is  `a.d` and `a0` is `a.x`
> 
> ```
> $ riscv64-unknown-linux-gnu-g++  -S -O2 -x c++ t.c -o-
> ...
> _Z3foo1A:
> .LFB0:
>         .cfi_startproc
>         fneg.d  fa0,fa0
>         addi    sp,sp,-16
>         .cfi_def_cfa_offset 16
>         addi    a0,a0,1
>         addi    sp,sp,16
>         .cfi_def_cfa_offset 0
>         tail    _Z3bar1A
>         .cfi_endproc
> ```
> 
> This is a bit worrying as it might complicate interoperability between C and C++ (I tried wrapping everything inside an `extern "C"` just in case but it didn't change g++'s behaviour).
> 
> Do you mind to confirm this issue?
Thanks, I'm seeing this in GCC 9.1.0 as well. I left[ a comment](https://github.com/riscv/riscv-elf-psabi-doc/issues/99#issuecomment-509233798) on the relevant psABI issue. It seems there is a GCC bug here, but hopefully someone can confirm what the "correct" behaviour is.


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

https://reviews.llvm.org/D60456