<div dir="ltr">Hi Christian,<div><br></div><div>Thanks for working on this! The patch looks good in general, so below are just the remaining indentation/coding style niggles.</div><div><br></div><div><div>     aarch64, // AArch64: aarch64</div>
<div>+    aarch64_be, // AArch64 (big endian): aarch64_be</div></div><div><br></div><div>Could you please then add a corresponding "(little endian)" comment to the line above?</div><div><br></div><div><div>@@ -20,6 +20,7 @@</div>
<div>   case UnknownArch: return "unknown";</div><div> </div><div>   case aarch64: return "aarch64";</div><div>+  case aarch64_be:return "aarch64_be";</div><div>   case arm:     return "arm";</div>
<div>   case hexagon: return "hexagon";</div><div>   case mips:    return "mips";</div></div><div><br></div><div>+  case aarch64_be:return "aarch64";<br></div><div><br></div><div>Should be a space after aarch64_be: (may have to add spaces to other lines to get them all to line up).</div>
<div><br></div><div><div>-AArch64Subtarget::AArch64Subtarget(StringRef TT, StringRef CPU, StringRef FS)</div><div>+AArch64Subtarget::AArch64Subtarget(StringRef TT, StringRef CPU, StringRef FS, bool little)</div><div>     : AArch64GenSubtargetInfo(TT, CPU, FS), HasFPARMv8(false), HasNEON(false),</div>
<div>-      HasCrypto(false), TargetTriple(TT), CPUString(CPU) {</div><div>+      HasCrypto(false), TargetTriple(TT), CPUString(CPU), IsLittle(little) {</div><div> </div></div><div>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?</div>
<div><br></div><div><div>+  /// IsLittle - The target is Little Endian</div><div>+  bool IsLittle;</div></div><div><br></div><div>Same here, and in many other places.</div><div><br></div><div>+AArch64ELFMCAsmInfo::AArch64ELFMCAsmInfo(StringRef TT) {<br>
</div><div><br></div><div>Given the caller has already converted TT into a Triple, it makes more sense to pass the Triple here than the string.</div><div><br></div><div><div>+MCAsmBackend *createAArch64beAsmBackend(const Target &T,</div>
<div>+                                      const MCRegisterInfo &MRI,</div><div>+                                      StringRef TT, StringRef CPU);</div><div>+</div></div><div><br></div><div>Indentation fail here.</div>
<div><br></div><div>Cheers,</div><div><br></div><div>James</div></div><div class="gmail_extra"><br><br><div class="gmail_quote">On 20 February 2014 16:04, Christian Pirker <span dir="ltr"><<a href="mailto:cpirker@a-bix.com" target="_blank">cpirker@a-bix.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi all,<br>
<br>
This patch adds a target machine class for little and big endian AArch64.<br>
This includes a modification to the data layout string.<br>
(A corresponding target description has been committed to cfe-commits.)<br>
Please review.<br>
<br>
Thanks,<br>
Christian<br>
<br>
Modified:<br>
        include/llvm/ADT/Triple.h<br>
        lib/Support/Triple.cpp<br>
        lib/Target/AArch64/<u></u>AArch64AsmPrinter.cpp<br>
        lib/Target/AArch64/<u></u>AArch64Subtarget.cpp<br>
        lib/Target/AArch64/<u></u>AArch64Subtarget.h<br>
        lib/Target/AArch64/<u></u>AArch64TargetMachine.cpp<br>
        lib/Target/AArch64/<u></u>AArch64TargetMachine.h<br>
        lib/Target/AArch64/AsmParser/<u></u>AArch64AsmParser.cpp<br>
        lib/Target/AArch64/<u></u>Disassembler/<u></u>AArch64Disassembler.cpp<br>
        lib/Target/AArch64/<u></u>MCTargetDesc/<u></u>AArch64AsmBackend.cpp<br>
        lib/Target/AArch64/<u></u>MCTargetDesc/<u></u>AArch64ELFObjectWriter.cpp<br>
        lib/Target/AArch64/<u></u>MCTargetDesc/AArch64MCAsmInfo.<u></u>cpp<br>
        lib/Target/AArch64/<u></u>MCTargetDesc/AArch64MCAsmInfo.<u></u>h<br>
        lib/Target/AArch64/<u></u>MCTargetDesc/<u></u>AArch64MCTargetDesc.cpp<br>
        lib/Target/AArch64/<u></u>MCTargetDesc/<u></u>AArch64MCTargetDesc.h<br>
        lib/Target/AArch64/TargetInfo/<u></u>AArch64TargetInfo.cpp<br>
<br>
<br>_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
<br></blockquote></div><br></div>