[PATCH] D27913: [XRay] Fix assertion failure on empty machine basic blocks (PR 31424)

[Chandler Carruth via Phabricator via llvm-commits]

chandlerc added a comment.

Also good to add an IR test case with 'unreachable' just for more coverage of these kinds of edge cases?



================
Comment at: lib/CodeGen/XRayInstrumentation.cpp:61-62
 
-void XRayInstrumentation::replaceRetWithPatchableRet(MachineFunction &MF,
-  const TargetInstrInfo *TII)
-{
+void XRayInstrumentation::replaceRetWithPatchableRet(
+    MachineFunction &MF, const TargetInstrInfo *TII) {
   // We look for *all* terminators and returns, then replace those with
----------------
Unrelated formatting change, please land in a separate commit.


================
Comment at: lib/CodeGen/XRayInstrumentation.cpp:93-94
 
-void XRayInstrumentation::prependRetWithPatchableExit(MachineFunction &MF,
-  const TargetInstrInfo *TII)
-{
+void XRayInstrumentation::prependRetWithPatchableExit(
+    MachineFunction &MF, const TargetInstrInfo *TII) {
   for (auto &MBB : MF) {
----------------
Ditto.


================
Comment at: lib/CodeGen/XRayInstrumentation.cpp:114-115
 bool XRayInstrumentation::runOnMachineFunction(MachineFunction &MF) {
+  if (MF.empty())
+    return false; // The function is empty.
+
----------------
Add a test case for this as well?


================
Comment at: lib/CodeGen/XRayInstrumentation.cpp:136-140
+    // It could be that the first MachineBasicBlock is empty, despite the
+    // function not being empty. So we insert the instruction in this basic
+    // block always.
+    BuildMI(FirstMBB, FirstMBB.begin(), DebugLoc(),
+            TII->get(TargetOpcode::PATCHABLE_FUNCTION_ENTER));
----------------
Why do you care about instrumenting functions which have no instructions in them?


https://reviews.llvm.org/D27913