[PATCH] D94143: [AArch64] Add support for the GNU ILP32 ABI

[Joel Jones via Phabricator via llvm-commits]

joelkevinjones added a comment.

In D16213 <https://reviews.llvm.org/D16213> I introduced the `MCTargetOptions` argument to the `MCAsmBackend` constructor, as well as in constructor wrapper functions. This change touched multiple (every, probably) backend. Given the extension to embed the ABI in the Triple, can the `MCTargetOptions` argument be removed? That shouldn't be in this commit, I think.

I have't checked the new test cases for completeness. I also haven't checked whether there are existing test cases not in this commit that might usefully have "`-target-abi=ilp32`" changed to "`-triple aarch64-none-linux-gnu_ilp32`"



================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:6952
 
   // void *__stack at offset 0
+  unsigned Offset = 0;
----------------
For all the following comments of the form:
`// ... at offset n`
perhaps change it to:
`// ... at offset m * pointer size`
so, for example, the next comment would be
`// void *__gr_top at offset 1 * pointer size`


================
Comment at: llvm/lib/Target/AArch64/AArch64TargetMachine.cpp:235
+  return Ret;
 }
 
----------------
I may be over using "?:", but I think using Explaining Variables (AKA [[ https://refactoring.com/catalog/extractVariable.html | Extract Variable ]]) makes the code clearer.
```
std::string Endian(LittleEndian ? "e" : "E");
std::string Ptr32(TT.getEnvironment == Triple::GNUILP32 ? "-p:32:32" : "");
return Endian + "-m:e" + Ptr32 + "-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128";
```


================
Comment at: llvm/test/CodeGen/AArch64/ilp32-tlsdesc.ll:22
+; CHECK-RELOC: R_AARCH64_P32_TLSDESC_CALL
+}
----------------
Should the `CHECK-RELOC` lines contain more complete checks like `arm32-elf-relocs.s`, for example, does? That is symbol name and offset.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94143