[PATCH] D102582: [RISCV] Report an error when ABI mismatch with target-abi module flag.

[Jessica Clarke via Phabricator via cfe-commits]

jrtc27 added inline comments.


================
Comment at: llvm/test/CodeGen/RISCV/module-target-abi-tests.ll:5
+; RUN: cat %s > %t.emptyabi
+; RUN: echo '!0 = !{i32 1, !"target-abi", !""}' >> %t.emptyabi
+; RUN: llc -mtriple=riscv32 < %t.emptyabi -o /dev/null
----------------
khchen wrote:
> jrtc27 wrote:
> > khchen wrote:
> > > khchen wrote:
> > > > luismarques wrote:
> > > > > Is this something that we are handling in general, having such flag without a value?
> > > > Good cached, in general the target-abi is not empty, I updated the current implementation, thanks!
> > > @luismarques 
> > > 
> > > Sorry, I forget that the empty target-abi are coming from some clang cc1 tests. 
> > > They are missing -target-abi option in clang cc1 so target-abi module flag is empty.
> > >  
> > > ```
> > > CodeGen/RISCV/riscv-atomics.c
> > > CodeGen/RISCV/riscv-inline-asm-rvv.c
> > > CodeGen/RISCV/riscv-inline-asm-xsfvfhbfmin.c
> > > CodeGen/RISCV/riscv-inline-asm.c
> > > ```
> > > 
> > > Maybe we need to calculate the default target-abi if it's empty? or handle empty target-abi in the backend?
> > > 
> > I do think we should be filling in the default ABI here, otherwise it's very fragile.
> Do you mean cc1 need to calculate the default target-abi and fill it in IR? 
Yes. Otherwise if Clang and LLVM ever disagree on the default ABI for any particular case then you will get inconsistent code, as the C source-visible ABI will use one but the code generation will use another. It also means you can never change the default ABI in LLVM since otherwise you will break existing bitcode files that rely on the default.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102582