[Hexagon] Incorrect MIR after "hexinsert" pass

[quic-akaryaki]

The "hexinsert" pass tries to replace bitfield operations with Hexagon insert instructions. To limit memory usage, there is a limit on the internal table size. When the limit is reached, the state is not correctly cleaned up so defs from from new insert instructions are not deleted after processing the basic block. Later, these defs can be incorrectly used in other basic blocks even they are not reachable. Then compiler will crash with:

*** Bad machine code: Virtual register defs don't dominate all uses. ***

Fixes: #163774

[llvmbot]

@llvm/pr-subscribers-backend-hexagon

Author: Alexey Karyakin (quic-akaryaki)

Changes

The "hexinsert" pass tries to replace bitfield operations with Hexagon insert instructions. To limit memory usage, there is a limit on the internal table size. When the limit is reached, the state is not correctly cleaned up so defs from from new insert instructions are not deleted after processing the basic block. Later, these defs can be incorrectly used in other basic blocks even they are not reachable. Then compiler will crash with:

*** Bad machine code: Virtual register defs don't dominate all uses. ***

Fixes: #163774

---

Full diff: https://github.com/llvm/llvm-project/pull/164021.diff

2 Files Affected:

• (modified) llvm/lib/Target/Hexagon/HexagonGenInsert.cpp (+6-2)
• (added) llvm/test/CodeGen/Hexagon/insert-big.ll (+36)

diff --git a/llvm/lib/Target/Hexagon/HexagonGenInsert.cpp b/llvm/lib/Target/Hexagon/HexagonGenInsert.cpp
index 4ddbe7a53701b..ff876f6595350 100644
--- a/llvm/lib/Target/Hexagon/HexagonGenInsert.cpp
+++ b/llvm/lib/Target/Hexagon/HexagonGenInsert.cpp
@@ -920,6 +920,10 @@ void HexagonGenInsert::collectInBlock(MachineBasicBlock *B,
   // successors have been processed.
   RegisterSet BlockDefs, InsDefs;
   for (MachineInstr &MI : *B) {
+    // Stop if the map size is too large.
+    if (IFMap.size() >= MaxIFMSize)
+      break;
+
     InsDefs.clear();
     getInstrDefs(&MI, InsDefs);
     // Leave those alone. They are more transparent than "insert".
@@ -942,8 +946,8 @@ void HexagonGenInsert::collectInBlock(MachineBasicBlock *B,
 
         findRecordInsertForms(VR, AVs);
         // Stop if the map size is too large.
-        if (IFMap.size() > MaxIFMSize)
-          return;
+        if (IFMap.size() >= MaxIFMSize)
+          break;
       }
     }
 
diff --git a/llvm/test/CodeGen/Hexagon/insert-big.ll b/llvm/test/CodeGen/Hexagon/insert-big.ll
new file mode 100644
index 0000000000000..fe3ec6f79546d
--- /dev/null
+++ b/llvm/test/CodeGen/Hexagon/insert-big.ll
@@ -0,0 +1,36 @@
+; Check that llc does not abort, which happened due to incorrect MIR.
+; RUN: llc -O2 -mtriple=hexagon -insert-max-ifmap=1 < %s
+; RUN: llc -O2 -mtriple=hexagon -insert-max-ifmap=2 < %s
+; RUN: llc -O2 -mtriple=hexagon -insert-max-ifmap=3 < %s
+; RUN: llc -O2 -mtriple=hexagon -insert-max-ifmap=4 < %s
+; RUN: llc -O2 -mtriple=hexagon -insert-max-ifmap=5 < %s
+
+define i32 @f(i32 %0, i32 %1, i32 %2) {
+entry:
+  switch i32 %0, label %common.ret1 [
+    i32 8907, label %3
+    i32 4115, label %6
+  ]
+
+common.ret1:
+  %common.ret1.op = phi i32 [ %5, %3 ], [ %526, %6 ], [ 0, %entry ]
+  ret i32 %common.ret1.op
+
+3:
+  %4 = shl i32 %2, 5
+  %5 = and i32 %4, 992
+  br label %common.ret1
+
+6:
+  %7 = shl i32 %0, 10
+  %8 = and i32 %7, 7168
+  %9 = shl i32 %0, 5
+  %10 = and i32 %9, 992
+  %11 = or i32 %10, %8
+  %12 = and i32 %0, 1
+  %13 = or i32 %11, %12
+  %14 = shl i32 %1, 1
+  %15 = and i32 %14, 2031616
+  %526 = or i32 %13, %15
+  br label %common.ret1
+}

[quic-akaryaki]

This does not fix "remark: Misaligned constant address: ...", maybe I should remove "Fixes" or create a separate issue for it?

[androm3da]

This does not fix "remark: Misaligned constant address: ...", maybe I should remove "Fixes" or create a separate issue for it?

Probably make a separate issue. Is the presence of that remark an indication of a defect?

[androm3da]

I can't reproduce the bug in the IR test case after this change. LGTM but someone else should approve.