[PATCH] D38102: [XRay] support conditional return on PPC.

Dean Michael Berris via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Sep 21 14:37:30 PDT 2017


dberris accepted this revision.
dberris added a comment.

Still LGTM, with one last suggestion.



================
Comment at: llvm/lib/CodeGen/XRayInstrumentation.cpp:205
     // For the architectures which don't have a single return instruction
-    prependRetWithPatchableExit(MF, TII);
+    prependRetWithPatchableExit(MF, TII, {false, true});
+    break;
----------------
Looking at this, I'm inclined to suggest that using an `enum class` for spelling out whether to handle tail calls might be more readable. Right now I have to see what the meaning of 'false' in the first position is. Something like:

```
prependRetWithPatchableExit(MF, TII, {InstrumentationOptions::TailCalls::IGNORE, InstrumentationOptions::Returns::HANDLE_ALL});
```

The enum class may or may not be nested in InstrumentationOptions, so I'll leave that up to you.


================
Comment at: llvm/test/CodeGen/PowerPC/xray-ret-is-terminator.ll:3
+
+define void @RetIsTerminator() #0 {
+; CHECK-LABEL @RetIsTerminator
----------------
nit: Opportunity here for "IllBeBack". 😛 


https://reviews.llvm.org/D38102





More information about the llvm-commits mailing list