xen0n marked 5 inline comments as done.
xen0n added inline comments.

================
Comment at: clang/lib/Driver/ToolChains/Arch/LoongArch.cpp:66
+  default:
+    return IsLA32 ? "ilp32d" : "lp64d";
+  }
----------------
xry111 wrote:
> SixWeining wrote:
> > Better to be `f`? (Probably most 32-bit hardwares do not support 
> > double-float? But I'm not sure about this.)
> The ISA manual says 64-bit float support is not limited by 32-bit basic ISA 
> support (except the moving instructions accessing 64-bit GPR).
AFAIK the consensus (even inside Loongson) is that even LA32's unmarked/default 
ABI is going to have hard F64 available. And yes the ISA manual is consistent 
with this view too. (IMO LA32 should instead default to soft-float, but if 
Loongson's commercial roadmap can guarantee hardware FPU availability on all 
meaningful LA32 models then that would backfire, who knows...)


================
Comment at: clang/lib/Driver/ToolChains/Linux.cpp:41
 /// so we provide a rough mapping here.
 std::string Linux::getMultiarchTriple(const Driver &D,
                                       const llvm::Triple &TargetTriple,
----------------
SixWeining wrote:
> xen0n wrote:
> > SixWeining wrote:
> > > Missing test. Perhaps add some in `clang/test/Driver/linux-ld.c` and 
> > > `clang/test/Driver/linux-header-search.cpp`? Or postpone this change 
> > > until loongarch is upstreamed to Debian?
> > Thanks, I'll try adding the tests in the next revision.
> > 
> > But given [[ https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1028654 | 
> > the recent reversal of upstream dpkg support ]] (apparently due to 
> > miscommunication) it may be prudent to wait until a consensus is reached 
> > Debian-side.
> [[ https://git.dpkg.org/cgit/dpkg/dpkg.git/commit/?id=9dfffbbae | The 
> multiarch tuples have been upstreamed to dpkg ]].  I think we can amend the 
> change and move on now.
Sorry for the delay in processing this, I've updated the code to reflect the 
latest spec change. I'll still have to check the unit-test part though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142688

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

Reply via email to