[PATCH] D70116: [RISCV] add subtargets initialized with target feature

[Kuan Hsu Chen (Zakk) via Phabricator via llvm-commits]

khchen added a comment.

In D70116#1761610 <https://reviews.llvm.org/D70116#1761610>, @lenary wrote:

> In D70116#1761541 <https://reviews.llvm.org/D70116#1761541>, @khchen wrote:
>
> > In D70116#1761492 <https://reviews.llvm.org/D70116#1761492>, @lenary wrote:
> >
> > > So am I right in thinking that neither `foo` nor `foo2` will trigger the `ABI_Unknown` issue, and the issue is only coming from hand-written assembly files?
> > >
> > > Actually, you may need to specify `-target-abi` in the top of the `subtarget-features-std-ext.ll` file.
> >
> >
> > Sorry I didn't explain clearly,
> >  The problem is caused by when I try to enable LTO and it need to pass `mabi` to LTO code generator (mabi is not encoded in bitcode)
> >  like we compile below case with `llc example.c -mtriple=riscv32 -mabi=ilp32f`.
> >
> >   define float @foo(i32 %a) nounwind #0 {
> >     %conv = sitofp i32 %a to float
> >     ret float %conv
> >   }
> >  
> >   define float @foo2(i32 %a) nounwind #1{
> >     %conv = sitofp i32 %a to float
> >     ret float %conv
> >   }
> >  
> >   attributes #0 = { "target-features"="+f"}
> >   attributes #1 = { "target-features"="+f"}
> >
> >
> > and they will show two error messages `Hard-float 'f' ABI can't be used for a target that doesn't support the F instruction set extension`.
> >  so there is no any issue in `subtarget-features-std-ext.ll` without -target-abi.
> >
> > when I started to fix this error, I found backend shares the ABI parsing/checking function (`computeTargetABI`) in codegen and MC layer when instance RISCVAsmBackend (note: IR function is not available at this point)
> >  ref. https://github.com/llvm/llvm-project/commit/fea4957177315f83746dca90cb4c9013eb465c46
> >
> > so this patch is workable, but it's not useful for LTO, not useful enough to support per function feature in backend, and then I'm not sure it's worth to upstream this patch.
>
>
> This patch seems to solve the issue of working out the correct Subtarget on a per-function basis? This would suggest it is useful.


Yes. Okay, agree with you.

> Presumably the next steps are to work out how to pass `-target-abi` through to the LTO backend via attributes? I was trying to see how other backends did this, but I'm not sure. I think the right way may be to have a feature per ABI, like ARM does here: https://github.com/llvm/llvm-project/blob/2d739f98d8a53e38bf9faa88cdb6b0c2a363fb77/clang/lib/Driver/ToolChains/Arch/ARM.cpp#L313-L319
> 
> It might be worth sending an email to llvm-dev, asking for guidance.

my original plan is assume there is only one ABI in one compilation unit, so just pass `-target-abi` to LTO backend via option, and fix the above issues I meet.
But if in the future the object file allow different functions has different ABIs in one compilation unit, it should be have per function ABI feature in backend.
I noted there is no one target looks `soft-float-abi` attribute to decide ABI, they almost uses `soft-float`.
so if rv32imafc/ilp32 combination (like arm's softfp) is illegal, I think using `soft-float` is enough and a good ideal.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70116