lenary added inline comments.

================
Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:1547
+  else if (IsRV64)
+    MArch = "rv64i";
+  else
----------------
lenary wrote:
> khchen wrote:
> > lenary wrote:
> > > I think this line is the issue: where someone doesn't specify `-march`, 
> > > you choose a default `-march` that does not appear in the list of 
> > > multilib arches in `RISCVMultilibSet` above.
> > > 
> > > I think this issue probably didn't arise when you had `rv64i/lp64` in the 
> > > list, but given that's not in the riscv-gcc `t-elf-multilib`, so this 
> > > code should choose a new default. I don't know how this new default will 
> > > affect defaults chosen in other parts of the clang codebase.
> > oh it's why I added rv64i/lp64 in previous version, but it's wrong default.
> > in riscv gcc if configure only with --enable-multilib, the default 
> > march/abi is rv64imafdc/lp64d
> > The option 1 is aligning to gcc, but the default abi is lp64 in clang,
> > maybe chosing rv64imac as default is less side effect? what do you think?
> The solution for this patch is to use the function `riscv::getRISCVABI(const 
> ArgList &Args, const llvm::Triple &Triple)` from 
> `clang/lib/Driver/ToolChains/Arch/RISCV.h`. 
> 
> Then when I update the logic there to match gcc's logic, the right thing will 
> happen. I will submit a patch to have this function match the gcc logic 
> today. It took me a few hours but I found the default logic for GCC. 
Sorry, I'm wrong. I'm adding a `riscv::getRISCVArch(...)` to that file, which 
will solve this issue, sorry for the confusion. Will add this patch as 
dependent on that one when it's ready.


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

https://reviews.llvm.org/D67508



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to