[PATCH] D145883: [Flang][RISCV] Emit target features for RISC-V

Mon Mar 13 14:20:53 PDT 2023

kiranchandramohan accepted this revision.
kiranchandramohan added a comment.

LG. Please wait for and OK from @jrtc27.

================
Comment at: clang/lib/Driver/ToolChains/Flang.cpp:112-114
+  case llvm::Triple::riscv64:
+    getTargetFeatures(D, Triple, Args, CmdArgs, /*ForAs*/ false);
+    break;
----------------
jrtc27 wrote:
> kiranchandramohan wrote:
> > jrtc27 wrote:
> > > mnadeem wrote:
> > > > identical code, could just do a fallthrough
> > > Why is this even conditional??
> > There could be other ways to do this. But it provided a way to understand the list of supported architectures.
> Well RISC-V needs -target-abi so its presence in the list does nothing to say it's properly supported. Nor do I think that's a good argument aside from that. You know every architecture needs to have its target features communicated, so making it conditional is not the right technical approach. Documenting the supported architectures should be done another way; perhaps in the actual documentation that developers/users are going to read, rather than buried in code?....
Putting the information in documentation also risks the fact that developers might not see it. Ideally, some portion of the compiler should provide an error if the target or a particular abi is not supported. I think we can discuss this specific issue in a separate patch.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145883