[llvm-bugs] [Bug 31960] New: ARM, Hexagon, and MIPS emit return blocks for which MBB.isReturnBlock returns false

[via llvm-bugs]

https://bugs.llvm.org/show_bug.cgi?id=31960

            Bug ID: 31960
           Summary: ARM, Hexagon, and MIPS emit return blocks for which
                    MBB.isReturnBlock returns false
           Product: libraries
           Version: trunk
          Hardware: PC
                OS: Windows NT
            Status: NEW
          Severity: enhancement
          Priority: P
         Component: Backend: ARM
          Assignee: unassignedbugs at nondot.org
          Reporter: rnk at google.com
                CC: llvm-bugs at lists.llvm.org

Kyle noticed that there are many ways that MachineBasicBlock::isReturnBlock can
return false for blocks that actually contain epilogues. It would be nice if
this predicate could remain accurate after prologue/epilogue insertion, because
it informs machine block placement and other optimizations. This came up during
review here: https://reviews.llvm.org/D29153

In particular, the ARM load store optimizer turns tBX_RET pseudo-instrs into
tBX indirect branch instructions, which are not flagged with isReturn. Other
targets (Mips, Hexagon) also seem to have this problem. You can find blocks
ending in indirect branches that aren't returns with this patch:

diff --git a/lib/CodeGen/MachineBlockPlacement.cpp
b/lib/CodeGen/MachineBlockPlacement.cpp
index 8a57f00..2ecd083 100644
--- a/lib/CodeGen/MachineBlockPlacement.cpp
+++ b/lib/CodeGen/MachineBlockPlacement.cpp
@@ -2302,6 +2302,15 @@ bool
MachineBlockPlacement::runOnMachineFunction(MachineFunction &MF) {
   if (skipFunction(*MF.getFunction()))
     return false;

+  for (auto &MBB : MF) {
+    if (!MBB.isReturnBlock() && MBB.succ_empty() && !MBB.empty() &&
+        MBB.back().isIndirectBranch()) {
+      MBB.dump();
+      llvm_unreachable(
+          "found no-successor indirect branch not flagged as return");
+    }
+  }
+
   // Check for single-block functions and skip them.
   if (std::next(MF.begin()) == MF.end())
     return false;

Here's an example where the ARM load/store optimizer creates this
inconsistency:

$ echo 'define i32 @simpleframe(<6 x i32>* %p) #0 {
entry:
  %0 = load <6 x i32>, <6 x i32>* %p, align 16
  %1 = extractelement <6 x i32> %0, i32 0
  %2 = extractelement <6 x i32> %0, i32 1
  %3 = extractelement <6 x i32> %0, i32 2
  %4 = extractelement <6 x i32> %0, i32 3
  %5 = extractelement <6 x i32> %0, i32 4
  %6 = extractelement <6 x i32> %0, i32 5
  %add1 = add nsw i32 %1, %2
  %add2 = add nsw i32 %add1, %3
  %add3 = add nsw i32 %add2, %4
  %add4 = add nsw i32 %add3, %5
  %add5 = add nsw i32 %add4, %6
  ret i32 %add5
}
' | llc -mtriple=thumbv4t-none--eabi

It would be nice to clean this up eventually by adding more duplicate indirect
branch pseudo instructions to carry the isReturn flag.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-bugs/attachments/20170214/1bd6427d/attachment.html>