Add AArch64 big endian Target (aarch64_be)

James Molloy james at jamesmolloy.co.uk
Thu Feb 20 09:00:41 PST 2014


Hi Christian,

Thanks for working on this! The patch looks good in general, so below are
just the remaining indentation/coding style niggles.

     aarch64, // AArch64: aarch64
+    aarch64_be, // AArch64 (big endian): aarch64_be

Could you please then add a corresponding "(little endian)" comment to the
line above?

@@ -20,6 +20,7 @@
   case UnknownArch: return "unknown";

   case aarch64: return "aarch64";
+  case aarch64_be:return "aarch64_be";
   case arm:     return "arm";
   case hexagon: return "hexagon";
   case mips:    return "mips";

+  case aarch64_be:return "aarch64";

Should be a space after aarch64_be: (may have to add spaces to other lines
to get them all to line up).

-AArch64Subtarget::AArch64Subtarget(StringRef TT, StringRef CPU, StringRef
FS)
+AArch64Subtarget::AArch64Subtarget(StringRef TT, StringRef CPU, StringRef
FS, bool little)
     : AArch64GenSubtargetInfo(TT, CPU, FS), HasFPARMv8(false),
HasNEON(false),
-      HasCrypto(false), TargetTriple(TT), CPUString(CPU) {
+      HasCrypto(false), TargetTriple(TT), CPUString(CPU), IsLittle(little)
{

The parameter "little" isn't capitalised the same as the rest of the
parameters. Also, could it please be "LittleEndian" so as to be more
explicit/obvious?

+  /// IsLittle - The target is Little Endian
+  bool IsLittle;

Same here, and in many other places.

+AArch64ELFMCAsmInfo::AArch64ELFMCAsmInfo(StringRef TT) {

Given the caller has already converted TT into a Triple, it makes more
sense to pass the Triple here than the string.

+MCAsmBackend *createAArch64beAsmBackend(const Target &T,
+                                      const MCRegisterInfo &MRI,
+                                      StringRef TT, StringRef CPU);
+

Indentation fail here.

Cheers,

James


On 20 February 2014 16:04, Christian Pirker <cpirker at a-bix.com> wrote:

> Hi all,
>
> This patch adds a target machine class for little and big endian AArch64.
> This includes a modification to the data layout string.
> (A corresponding target description has been committed to cfe-commits.)
> Please review.
>
> Thanks,
> Christian
>
> Modified:
>         include/llvm/ADT/Triple.h
>         lib/Support/Triple.cpp
>         lib/Target/AArch64/AArch64AsmPrinter.cpp
>         lib/Target/AArch64/AArch64Subtarget.cpp
>         lib/Target/AArch64/AArch64Subtarget.h
>         lib/Target/AArch64/AArch64TargetMachine.cpp
>         lib/Target/AArch64/AArch64TargetMachine.h
>         lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp
>         lib/Target/AArch64/Disassembler/AArch64Disassembler.cpp
>         lib/Target/AArch64/MCTargetDesc/AArch64AsmBackend.cpp
>         lib/Target/AArch64/MCTargetDesc/AArch64ELFObjectWriter.cpp
>         lib/Target/AArch64/MCTargetDesc/AArch64MCAsmInfo.cpp
>         lib/Target/AArch64/MCTargetDesc/AArch64MCAsmInfo.h
>         lib/Target/AArch64/MCTargetDesc/AArch64MCTargetDesc.cpp
>         lib/Target/AArch64/MCTargetDesc/AArch64MCTargetDesc.h
>         lib/Target/AArch64/TargetInfo/AArch64TargetInfo.cpp
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140220/345d1863/attachment.html>


More information about the llvm-commits mailing list