[PATCH] D26412: [XRay] Support AArch64 in LLVM

Dean Michael Berris via llvm-commits llvm-commits at lists.llvm.org
Tue Nov 8 15:14:50 PST 2016


dberris requested changes to this revision.
dberris added inline comments.
This revision now requires changes to proceed.


================
Comment at: lib/Target/AArch64/AArch64AsmPrinter.cpp:159
+      "xray_instr_map", ELF::SHT_PROGBITS,
+      ELF::SHF_ALLOC | ELF::SHF_GROUP | ELF::SHF_MERGE, 0,
+      CurrentFnSym->getName());
----------------
I distinctly remember having to do something special for non-COMDAT sections, especially when doing section-per-function in x86. I suspect you'd want to replicate that logic here somehow, with a `//TODO` to merge the logic for ELF XRay sleds at a higher level for better re-use.


================
Comment at: lib/Target/AArch64/AArch64AsmPrinter.cpp:210-212
+  {
+    EmitToStreamer(*OutStreamer, MCInstBuilder(AArch64::HINT).addImm(0));
+  }
----------------
Probably remove '{}' for single line bodies?


================
Comment at: lib/Target/AArch64/AArch64Subtarget.h:176
 
+  virtual bool isXRaySupported() const override { return true; }
+
----------------
I don't think you need the virtual here especially since you mark it `override` anyway.


https://reviews.llvm.org/D26412





More information about the llvm-commits mailing list