[Lldb-commits] [PATCH] D128612: RISC-V big-endian support implementation

[Fangrui Song via Phabricator via lldb-commits]

MaskRay added a comment.

lld/ELF change should be dropped from this change. Don't use `config->endianness`.
I feel sad that for little-endian users who don't use big-endian, every write now is slightly slower due to a check ;-)



================
Comment at: clang/lib/Basic/Targets/RISCV.cpp:124
   Builder.defineMacro("__riscv");
-  bool Is64Bit = getTriple().getArch() == llvm::Triple::riscv64;
+  bool Is64Bit = (getTriple().getArch() == llvm::Triple::riscv64 ||
+                  getTriple().getArch() == llvm::Triple::riscv64be);
----------------
The convention doesn't add `()` in such an assignment.


================
Comment at: clang/lib/Basic/Targets/RISCV.cpp:220
 
-  if (getTriple().getArch() == llvm::Triple::riscv64) {
+  if (getTriple().getArch() == llvm::Triple::riscv64 ||
+      getTriple().getArch() == llvm::Triple::riscv64be) {
----------------
This can be simplified with something like `isRISCV64()`


================
Comment at: clang/lib/Basic/Targets/RISCV.h:144
+
+    StringRef LayoutEndianness = Triple.isLittleEndian() ? "e" : "E";
+
----------------
You may use a `char` and possibly fold this into the expression below.


================
Comment at: clang/lib/Basic/Targets/RISCV.h:145
+    StringRef LayoutEndianness = Triple.isLittleEndian() ? "e" : "E";
+
+    resetDataLayout(
----------------
delete blank line


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128612