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

[Guy Benyei via Phabricator via cfe-commits]

gbenyei created this revision.
gbenyei added a reviewer: asb.
gbenyei added projects: clang, LLVM, lld.
Herald added subscribers: Enna1, sunshaoce, VincentWu, luke957, StephenFan, 
vkmr, frasercrmck, luismarques, apazos, sameer.abuasal, s.egerton, Jim, ormris, 
jocewei, PkmX, the_o, brucehoult, MartinMosbeck, rogfer01, edward-jones, 
zzheng, jrtc27, shiva0217, kito-cheng, niosHD, sabuasal, simoncook, johnrusso, 
rbar, hiraditya, arichardson, mgorny, emaste.
Herald added a reviewer: alexander-shaposhnikov.
Herald added a reviewer: rupprecht.
Herald added a reviewer: jhenderson.
Herald added a reviewer: MaskRay.
Herald added a reviewer: aaron.ballman.
Herald added a project: All.
gbenyei requested review of this revision.
Herald added subscribers: lldb-commits, cfe-commits, pcwang-thead.
Herald added a project: LLDB.

Implement riscv32be and riscv64be targets.
The RISC-V big- and bi-endian targets are discussed in the RISC-V spec  Version 
20191213, but some aspects, like ABI are still unclear.
The instruction encoding is little endian in both big- and little-endian modes. 
ISA spec Volume 1 1.15: "Instructions are stored in memory as a sequence
of 16-bit little-endian parcels, regardless of memory system endianness".

RISC-V Big-endian cores are already supported by GCC. Where spec is unclear, we 
aim to be compatible with GCC.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D128612

Files:
  clang/include/clang/Basic/Attr.td
  clang/lib/Basic/Targets.cpp
  clang/lib/Basic/Targets/OSTargets.h
  clang/lib/Basic/Targets/RISCV.cpp
  clang/lib/Basic/Targets/RISCV.h
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/TargetInfo.cpp
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/Arch/RISCV.cpp
  clang/lib/Driver/ToolChains/BareMetal.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/lib/Driver/ToolChains/FreeBSD.cpp
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/lib/Driver/ToolChains/Linux.cpp
  clang/lib/Driver/ToolChains/RISCVToolchain.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  lld/ELF/Arch/RISCV.cpp
  lld/ELF/InputFiles.cpp
  lldb/source/Utility/ArchSpec.cpp
  llvm/cmake/config-ix.cmake
  llvm/cmake/config.guess
  llvm/include/llvm/ADT/Triple.h
  llvm/include/llvm/Object/ELFObjectFile.h
  llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
  llvm/lib/ExecutionEngine/JITLink/ELF.cpp
  llvm/lib/ExecutionEngine/JITLink/ELF_riscv.cpp
  llvm/lib/ExecutionEngine/Orc/EPCIndirectionUtils.cpp
  llvm/lib/ExecutionEngine/Orc/IndirectionUtils.cpp
  llvm/lib/ExecutionEngine/Orc/LazyReexports.cpp
  llvm/lib/Object/RelocationResolver.cpp
  llvm/lib/Support/Triple.cpp
  llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
  llvm/lib/Target/RISCV/Disassembler/RISCVDisassembler.cpp
  llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp
  llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.h
  llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCAsmInfo.cpp
  llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCTargetDesc.cpp
  llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp
  llvm/lib/Target/RISCV/RISCVTargetMachine.cpp
  llvm/lib/Target/RISCV/TargetInfo/RISCVTargetInfo.cpp
  llvm/lib/Target/RISCV/TargetInfo/RISCVTargetInfo.h
  llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
  llvm/tools/llvm-objcopy/ObjcopyOptions.cpp
  llvm/unittests/Object/ELFObjectFileTest.cpp

Index: llvm/unittests/Object/ELFObjectFileTest.cpp
===
--- llvm/unittests/Object/ELFObjectFileTest.cpp
+++ llvm/unittests/Object/ELFObjectFileTest.cpp
@@ -186,10 +186,10 @@
 }
 
 TEST(ELFObjectFileTest, MachineTestForRISCV) {
-  std::array Formats = {"elf32-littleriscv", "elf32-littleriscv",
-  "elf64-littleriscv", "elf64-littleriscv"};
-  std::array Archs = {Triple::riscv32, Triple::riscv32,
-   Triple::riscv64, Triple::riscv64};
+  std::array Formats = {"elf32-littleriscv", "elf32-bigriscv",
+  "elf64-littleriscv", "elf64-bigriscv"};
+  std::array Archs = {Triple::riscv32, Triple::riscv32be,
+   Triple::riscv64, Triple::riscv64be};
   size_t I = 0;
   for (const DataForTest &D : generateData(ELF::EM_RISCV)) {
 checkFormatAndArch(D, Formats[I], Archs[I]);
Index: llvm/tools/llvm-objcopy/ObjcopyOptions.cpp
===
--- llvm/tools/llvm-objcopy/ObjcopyOptions.cpp
+++ llvm/tools/llvm-objcopy/ObjcopyOptions.cpp
@@ -301,6 +301,8 @@
 // RISC-V
 {"elf32-littleriscv", {ELF::EM_RISCV, false, true}},
 {"elf64-littleriscv", {ELF::EM_RISCV, true, true}},
+{"elf32-bigriscv", {ELF::EM_RISCV, false, false}},
+{"elf64-bigriscv", {ELF::EM_RISCV, true, false}},
 // PowerPC
 {"elf32-powerpc", {ELF::EM_PPC, false, false}},
 {"elf32-powerpcle", {ELF::EM_PPC, fa

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

[Guy Benyei via Phabricator via cfe-commits]

gbenyei updated this revision to Diff 440196.
gbenyei added a comment.

Thanks, Craig. Updated the patch with your remarks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128612

Files:
  clang/include/clang/Basic/Attr.td
  clang/lib/Basic/Targets.cpp
  clang/lib/Basic/Targets/OSTargets.h
  clang/lib/Basic/Targets/RISCV.cpp
  clang/lib/Basic/Targets/RISCV.h
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/TargetInfo.cpp
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/Arch/RISCV.cpp
  clang/lib/Driver/ToolChains/BareMetal.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/lib/Driver/ToolChains/FreeBSD.cpp
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/lib/Driver/ToolChains/Linux.cpp
  clang/lib/Driver/ToolChains/RISCVToolchain.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  lld/ELF/Arch/RISCV.cpp
  lld/ELF/InputFiles.cpp
  lldb/source/Utility/ArchSpec.cpp
  llvm/cmake/config-ix.cmake
  llvm/cmake/config.guess
  llvm/include/llvm/ADT/Triple.h
  llvm/include/llvm/Object/ELFObjectFile.h
  llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
  llvm/lib/ExecutionEngine/JITLink/ELF.cpp
  llvm/lib/ExecutionEngine/JITLink/ELF_riscv.cpp
  llvm/lib/ExecutionEngine/Orc/EPCIndirectionUtils.cpp
  llvm/lib/ExecutionEngine/Orc/IndirectionUtils.cpp
  llvm/lib/ExecutionEngine/Orc/LazyReexports.cpp
  llvm/lib/Object/RelocationResolver.cpp
  llvm/lib/Support/Triple.cpp
  llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
  llvm/lib/Target/RISCV/Disassembler/RISCVDisassembler.cpp
  llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp
  llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.h
  llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCAsmInfo.cpp
  llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCTargetDesc.cpp
  llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp
  llvm/lib/Target/RISCV/RISCVTargetMachine.cpp
  llvm/lib/Target/RISCV/TargetInfo/RISCVTargetInfo.cpp
  llvm/lib/Target/RISCV/TargetInfo/RISCVTargetInfo.h
  llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
  llvm/tools/llvm-objcopy/ObjcopyOptions.cpp
  llvm/unittests/Object/ELFObjectFileTest.cpp

Index: llvm/unittests/Object/ELFObjectFileTest.cpp
===
--- llvm/unittests/Object/ELFObjectFileTest.cpp
+++ llvm/unittests/Object/ELFObjectFileTest.cpp
@@ -186,10 +186,10 @@
 }
 
 TEST(ELFObjectFileTest, MachineTestForRISCV) {
-  std::array Formats = {"elf32-littleriscv", "elf32-littleriscv",
-  "elf64-littleriscv", "elf64-littleriscv"};
-  std::array Archs = {Triple::riscv32, Triple::riscv32,
-   Triple::riscv64, Triple::riscv64};
+  std::array Formats = {"elf32-littleriscv", "elf32-bigriscv",
+  "elf64-littleriscv", "elf64-bigriscv"};
+  std::array Archs = {Triple::riscv32, Triple::riscv32be,
+   Triple::riscv64, Triple::riscv64be};
   size_t I = 0;
   for (const DataForTest &D : generateData(ELF::EM_RISCV)) {
 checkFormatAndArch(D, Formats[I], Archs[I]);
Index: llvm/tools/llvm-objcopy/ObjcopyOptions.cpp
===
--- llvm/tools/llvm-objcopy/ObjcopyOptions.cpp
+++ llvm/tools/llvm-objcopy/ObjcopyOptions.cpp
@@ -301,6 +301,8 @@
 // RISC-V
 {"elf32-littleriscv", {ELF::EM_RISCV, false, true}},
 {"elf64-littleriscv", {ELF::EM_RISCV, true, true}},
+{"elf32-bigriscv", {ELF::EM_RISCV, false, false}},
+{"elf64-bigriscv", {ELF::EM_RISCV, true, false}},
 // PowerPC
 {"elf32-powerpc", {ELF::EM_PPC, false, false}},
 {"elf32-powerpcle", {ELF::EM_PPC, false, true}},
Index: llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
===
--- llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
+++ llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
@@ -479,7 +479,8 @@
   bool IsMIPS64 = TargetTriple.isMIPS64();
   bool IsArmOrThumb = TargetTriple.isARM() || TargetTriple.isThumb();
   bool IsAArch64 = TargetTriple.getArch() == Triple::aarch64;
-  bool IsRISCV64 = TargetTriple.getArch() == Triple::riscv64;
+  bool IsRISCV64 = TargetTriple.getArch() == Triple::riscv64 ||
+   TargetTriple.getArch() == Triple::riscv64be;
   bool IsWindows = TargetTriple.isOSWindows();
   bool IsFuchsia = TargetTriple.isOSFuchsia();
   bool IsEmscripten = TargetTriple.isOSEmscripten();
Index: llvm/lib/Target/RISCV/TargetInfo/RISCVTargetInfo.h
===
--- llvm/lib/Target/RISCV/TargetInfo/RISCVTargetInfo.h
+++ llvm/lib/Target/RISCV/TargetInfo/RISCVTargetInfo.h
@@ -15,6 +15,8 @@
 
 Target &getTheRISCV32Target();
 Target &getTheRISCV64Target();
+Target &getTheRISCV32beTarget

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

[Guy Benyei via Phabricator via cfe-commits]

gbenyei updated this revision to Diff 440579.
gbenyei marked 7 inline comments as done.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128612

Files:
  clang/include/clang/Basic/Attr.td
  clang/lib/Basic/Targets.cpp
  clang/lib/Basic/Targets/OSTargets.h
  clang/lib/Basic/Targets/RISCV.cpp
  clang/lib/Basic/Targets/RISCV.h
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/TargetInfo.cpp
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/Arch/RISCV.cpp
  clang/lib/Driver/ToolChains/BareMetal.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/lib/Driver/ToolChains/FreeBSD.cpp
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/lib/Driver/ToolChains/Linux.cpp
  clang/lib/Driver/ToolChains/RISCVToolchain.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  lld/ELF/Arch/RISCV.cpp
  lld/ELF/InputFiles.cpp
  lldb/source/Utility/ArchSpec.cpp
  llvm/cmake/config.guess
  llvm/include/llvm/ADT/Triple.h
  llvm/include/llvm/Object/ELFObjectFile.h
  llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
  llvm/lib/ExecutionEngine/JITLink/ELF.cpp
  llvm/lib/ExecutionEngine/JITLink/ELF_riscv.cpp
  llvm/lib/ExecutionEngine/Orc/EPCIndirectionUtils.cpp
  llvm/lib/ExecutionEngine/Orc/IndirectionUtils.cpp
  llvm/lib/ExecutionEngine/Orc/LazyReexports.cpp
  llvm/lib/Object/RelocationResolver.cpp
  llvm/lib/Support/Triple.cpp
  llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
  llvm/lib/Target/RISCV/Disassembler/RISCVDisassembler.cpp
  llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp
  llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.h
  llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCAsmInfo.cpp
  llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCTargetDesc.cpp
  llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp
  llvm/lib/Target/RISCV/RISCVTargetMachine.cpp
  llvm/lib/Target/RISCV/TargetInfo/RISCVTargetInfo.cpp
  llvm/lib/Target/RISCV/TargetInfo/RISCVTargetInfo.h
  llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
  llvm/test/tools/llvm-objcopy/ELF/binary-output-target.test
  llvm/tools/llvm-objcopy/ObjcopyOptions.cpp
  llvm/unittests/Object/ELFObjectFileTest.cpp

Index: llvm/unittests/Object/ELFObjectFileTest.cpp
===
--- llvm/unittests/Object/ELFObjectFileTest.cpp
+++ llvm/unittests/Object/ELFObjectFileTest.cpp
@@ -186,10 +186,10 @@
 }
 
 TEST(ELFObjectFileTest, MachineTestForRISCV) {
-  std::array Formats = {"elf32-littleriscv", "elf32-littleriscv",
-  "elf64-littleriscv", "elf64-littleriscv"};
-  std::array Archs = {Triple::riscv32, Triple::riscv32,
-   Triple::riscv64, Triple::riscv64};
+  std::array Formats = {"elf32-littleriscv", "elf32-bigriscv",
+  "elf64-littleriscv", "elf64-bigriscv"};
+  std::array Archs = {Triple::riscv32, Triple::riscv32be,
+   Triple::riscv64, Triple::riscv64be};
   size_t I = 0;
   for (const DataForTest &D : generateData(ELF::EM_RISCV)) {
 checkFormatAndArch(D, Formats[I], Archs[I]);
Index: llvm/tools/llvm-objcopy/ObjcopyOptions.cpp
===
--- llvm/tools/llvm-objcopy/ObjcopyOptions.cpp
+++ llvm/tools/llvm-objcopy/ObjcopyOptions.cpp
@@ -301,6 +301,8 @@
 // RISC-V
 {"elf32-littleriscv", {ELF::EM_RISCV, false, true}},
 {"elf64-littleriscv", {ELF::EM_RISCV, true, true}},
+{"elf32-bigriscv", {ELF::EM_RISCV, false, false}},
+{"elf64-bigriscv", {ELF::EM_RISCV, true, false}},
 // PowerPC
 {"elf32-powerpc", {ELF::EM_PPC, false, false}},
 {"elf32-powerpcle", {ELF::EM_PPC, false, true}},
Index: llvm/test/tools/llvm-objcopy/ELF/binary-output-target.test
===
--- llvm/test/tools/llvm-objcopy/ELF/binary-output-target.test
+++ llvm/test/tools/llvm-objcopy/ELF/binary-output-target.test
@@ -33,6 +33,12 @@
 # RUN: llvm-objcopy -I binary -O elf64-littleriscv %t.txt %t.rv64.o
 # RUN: llvm-readobj --file-headers %t.rv64.o | FileCheck %s --check-prefixes=CHECK,LE,RISCV64,64
 
+# RUN: llvm-objcopy -I binary -O elf32-bigriscv %t.txt %t.rv32.o
+# RUN: llvm-readobj --file-headers %t.rv32.o | FileCheck %s --check-prefixes=CHECK,BE,RISCV32,32
+
+# RUN: llvm-objcopy -I binary -O elf64-bigriscv %t.txt %t.rv64.o
+# RUN: llvm-readobj --file-headers %t.rv64.o | FileCheck %s --check-prefixes=CHECK,BE,RISCV64,64
+
 # RUN: llvm-objcopy -I binary -O elf32-sparc %t.txt %t.sparc.o
 # RUN: llvm-readobj --file-headers %t.sparc.o | FileCheck %s --check-prefixes=CHECK,BE,SPARC,32
 
Index: llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
===
--- llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
+++ llvm/lib/T

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

[Guy Benyei via Phabricator via cfe-commits]

gbenyei added a comment.

In D128612#3617906 , @jhenderson 
wrote:

> Objcopy aspects look good, thanks.

Thanks




Comment at: llvm/lib/ExecutionEngine/JITLink/ELF_riscv.cpp:554
 .buildGraph();
-  } else {
-assert((*ELFObj)->getArch() == Triple::riscv32 &&
-   "Invalid triple for RISCV ELF object file");
+  } else if ((*ELFObj)->getArch() == Triple::riscv64be) {
+auto &ELFObjFile = cast>(**ELFObj);

jrtc27 wrote:
> Why switch to this order when before you've used 32, 64, 32be, 64be as the 
> order
The order in the code before my changes is 64, 32. I guess for no good reason, 
but I prefer not to re-order code while implementing a feature - it trashes git 
history.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128612

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

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

[Guy Benyei via Phabricator via cfe-commits]

gbenyei added a comment.

In D128612#3617955 , @MaskRay wrote:

> 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 ;-)

Hi, I'm not sure I get it. How will we have a fully functional toolchain, if I 
don't implement the lld/ELF part?
In LLVM, unlike in GCC, target related decisions happen in runtime. I think 
it's a high level design decision. While I can understand the pain of LE 
developers getting a slightly slower linker due to endianness checking, I sure 
will feel the pain of a BE developer not having a linker...

Please explain why I shouldn't use `config->endianness`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128612

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

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

[Guy Benyei via Phabricator via cfe-commits]

gbenyei marked 3 inline comments as done.
gbenyei added a comment.

In D128612#3620911 , @MaskRay wrote:

> In D128612#3618167 , @gbenyei wrote:
>
>> In D128612#3617955 , @MaskRay 
>> wrote:
>>
>>> 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 ;-)
>>
>> Hi, I'm not sure I get it. How will we have a fully functional toolchain, if 
>> I don't implement the lld/ELF part?
>> In LLVM, unlike in GCC, target related decisions happen in runtime. I think 
>> it's a high level design decision. While I can understand the pain of LE 
>> developers getting a slightly slower linker due to endianness checking, I 
>> sure will feel the pain of a BE developer not having a linker...
>>
>> Please explain why I shouldn't use `config->endianness`?
>
> See PPC64.cpp. See D96188  how I added 
> aarch64_be support. A set of representative tests should be picked with be 
> tests.
> If llvm-project consensus is that we will add big-endian support, I can 
> handle lld/ELF part. I am mostly concerned with this scenarios that some 
> RISC-V folks click LGTM, and the change lands with no test in some areas, or 
> the code somewhat breaks local convention.
>
> Many of the changes in this patch probably should be split. llvm-objcopy and 
> JIT changes definitely needs appropriate tests and the suitable domain 
> reviewers.

Thanks, it makes more sense now. I'll split the LLD changes, and remove the JIT 
related stuff.




Comment at: clang/lib/Basic/Targets/RISCV.h:144
+
+StringRef LayoutEndianness = Triple.isLittleEndian() ? "e" : "E";
+

MaskRay wrote:
> You may use a `char` and possibly fold this into the expression below.
Concatenating a conditional char and a string literal might be tricky, I'm not 
sure there is a cleaner solution.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128612

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

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

[Guy Benyei via Phabricator via cfe-commits]

gbenyei marked 3 inline comments as done.
gbenyei added inline comments.



Comment at: llvm/lib/ExecutionEngine/JITLink/ELF_riscv.cpp:554
 .buildGraph();
-  } else {
-assert((*ELFObj)->getArch() == Triple::riscv32 &&
-   "Invalid triple for RISCV ELF object file");
+  } else if ((*ELFObj)->getArch() == Triple::riscv64be) {
+auto &ELFObjFile = cast>(**ELFObj);

gbenyei wrote:
> jrtc27 wrote:
> > Why switch to this order when before you've used 32, 64, 32be, 64be as the 
> > order
> The order in the code before my changes is 64, 32. I guess for no good 
> reason, but I prefer not to re-order code while implementing a feature - it 
> trashes git history.
Removed this part anyway - JIT is out of scope for this commit. Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128612

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

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

[Guy Benyei via Phabricator via cfe-commits]

gbenyei updated this revision to Diff 441410.
gbenyei added a comment.

Removed LLD and JIT related parts - JIT is out of my scope, and LLD will be in 
an additional patch.
Fixed additional remarks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128612

Files:
  clang/include/clang/Basic/Attr.td
  clang/lib/Basic/Targets.cpp
  clang/lib/Basic/Targets/OSTargets.h
  clang/lib/Basic/Targets/RISCV.cpp
  clang/lib/Basic/Targets/RISCV.h
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/TargetInfo.cpp
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/Arch/RISCV.cpp
  clang/lib/Driver/ToolChains/BareMetal.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/lib/Driver/ToolChains/FreeBSD.cpp
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/lib/Driver/ToolChains/Linux.cpp
  clang/lib/Driver/ToolChains/RISCVToolchain.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  llvm/cmake/config.guess
  llvm/include/llvm/ADT/Triple.h
  llvm/include/llvm/Object/ELFObjectFile.h
  llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
  llvm/lib/Object/RelocationResolver.cpp
  llvm/lib/Support/Triple.cpp
  llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
  llvm/lib/Target/RISCV/Disassembler/RISCVDisassembler.cpp
  llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp
  llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.h
  llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCAsmInfo.cpp
  llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCTargetDesc.cpp
  llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp
  llvm/lib/Target/RISCV/RISCVTargetMachine.cpp
  llvm/lib/Target/RISCV/TargetInfo/RISCVTargetInfo.cpp
  llvm/lib/Target/RISCV/TargetInfo/RISCVTargetInfo.h
  llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
  llvm/test/tools/llvm-objcopy/ELF/binary-output-target.test
  llvm/tools/llvm-objcopy/ObjcopyOptions.cpp
  llvm/unittests/Object/ELFObjectFileTest.cpp

Index: llvm/unittests/Object/ELFObjectFileTest.cpp
===
--- llvm/unittests/Object/ELFObjectFileTest.cpp
+++ llvm/unittests/Object/ELFObjectFileTest.cpp
@@ -186,10 +186,10 @@
 }
 
 TEST(ELFObjectFileTest, MachineTestForRISCV) {
-  std::array Formats = {"elf32-littleriscv", "elf32-littleriscv",
-  "elf64-littleriscv", "elf64-littleriscv"};
-  std::array Archs = {Triple::riscv32, Triple::riscv32,
-   Triple::riscv64, Triple::riscv64};
+  std::array Formats = {"elf32-littleriscv", "elf32-bigriscv",
+  "elf64-littleriscv", "elf64-bigriscv"};
+  std::array Archs = {Triple::riscv32, Triple::riscv32be,
+   Triple::riscv64, Triple::riscv64be};
   size_t I = 0;
   for (const DataForTest &D : generateData(ELF::EM_RISCV)) {
 checkFormatAndArch(D, Formats[I], Archs[I]);
Index: llvm/tools/llvm-objcopy/ObjcopyOptions.cpp
===
--- llvm/tools/llvm-objcopy/ObjcopyOptions.cpp
+++ llvm/tools/llvm-objcopy/ObjcopyOptions.cpp
@@ -301,6 +301,8 @@
 // RISC-V
 {"elf32-littleriscv", {ELF::EM_RISCV, false, true}},
 {"elf64-littleriscv", {ELF::EM_RISCV, true, true}},
+{"elf32-bigriscv", {ELF::EM_RISCV, false, false}},
+{"elf64-bigriscv", {ELF::EM_RISCV, true, false}},
 // PowerPC
 {"elf32-powerpc", {ELF::EM_PPC, false, false}},
 {"elf32-powerpcle", {ELF::EM_PPC, false, true}},
Index: llvm/test/tools/llvm-objcopy/ELF/binary-output-target.test
===
--- llvm/test/tools/llvm-objcopy/ELF/binary-output-target.test
+++ llvm/test/tools/llvm-objcopy/ELF/binary-output-target.test
@@ -33,6 +33,12 @@
 # RUN: llvm-objcopy -I binary -O elf64-littleriscv %t.txt %t.rv64.o
 # RUN: llvm-readobj --file-headers %t.rv64.o | FileCheck %s --check-prefixes=CHECK,LE,RISCV64,64
 
+# RUN: llvm-objcopy -I binary -O elf32-bigriscv %t.txt %t.rv32.o
+# RUN: llvm-readobj --file-headers %t.rv32.o | FileCheck %s --check-prefixes=CHECK,BE,RISCV32,32
+
+# RUN: llvm-objcopy -I binary -O elf64-bigriscv %t.txt %t.rv64.o
+# RUN: llvm-readobj --file-headers %t.rv64.o | FileCheck %s --check-prefixes=CHECK,BE,RISCV64,64
+
 # RUN: llvm-objcopy -I binary -O elf32-sparc %t.txt %t.sparc.o
 # RUN: llvm-readobj --file-headers %t.sparc.o | FileCheck %s --check-prefixes=CHECK,BE,SPARC,32
 
Index: llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
===
--- llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
+++ llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
@@ -480,7 +480,8 @@
   bool IsMIPS64 = TargetTriple.isMIPS64();
   bool IsArmOrThumb = TargetTriple.isARM() || TargetTriple.isThumb();
   bool IsAArch64 = TargetTriple.getArc