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

[Dean Michael Berris via llvm-commits]

dberris accepted this revision.
dberris added a comment.
This revision is now accepted and ready to land.

Just a couple style issues, thanks @rSerge -- I'll look forward to the refactoring afterwards to consolidate some of the ELF-specific XRay instrumentation stuff potentially at a higher level at a later time.



================
Comment at: lib/Target/AArch64/AArch64AsmPrinter.cpp:162-170
+    if (Fn->hasComdat()) {
+      Section = OutContext.getELFSection("xray_instr_map", ELF::SHT_PROGBITS,
+        ELF::SHF_ALLOC | ELF::SHF_GROUP, 0,
+        Fn->getComdat()->getName());
+    }
+    else {
+      Section = OutContext.getELFSection("xray_instr_map", ELF::SHT_PROGBITS,
----------------
Since these are single-statement bodies for the true and false branches, LLVM style seems to prefer not having curlies around them:

```
if (...)
  Section = ...;
else
  Section = ...;
```

Also, if you do need compound statements in the branches, the `else` should be on the same line as the previous `}`. So:

```
if (...) {
  ...
} else {
  ...
}
```


================
Comment at: lib/Target/AArch64/AArch64AsmPrinter.cpp:685-686
+    LowerPATCHABLE_FUNCTION_ENTER(*MI);
+    return;
+  case TargetOpcode::PATCHABLE_FUNCTION_EXIT:
+    LowerPATCHABLE_FUNCTION_EXIT(*MI);
----------------
To keep it consistent with surrounding style, maybe add an empty line between the `return;` and the `case ...:`? i.e.

```
  case TargetOpcode::...:
    ...
    return;

  case TargetOpcode::...;
    ...
    return;
```


https://reviews.llvm.org/D26412