-
Notifications
You must be signed in to change notification settings - Fork 15k
release/21.x: [Hexagon] Incorrect MIR after "hexinsert" pass (#164021) #164354
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: release/21.x
Are you sure you want to change the base?
Conversation
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: llvm#163774 (cherry picked from commit f3a60cf)
|
@androm3da What do you think about merging this PR to the release branch? |
|
@llvm/pr-subscribers-backend-hexagon Author: None (llvmbot) ChangesBackport f3a60cf Requested by: @androm3da Full diff: https://github.com/llvm/llvm-project/pull/164354.diff 2 Files Affected:
diff --git a/llvm/lib/Target/Hexagon/HexagonGenInsert.cpp b/llvm/lib/Target/Hexagon/HexagonGenInsert.cpp
index a9201460d8e2e..2399e2a28eb22 100644
--- a/llvm/lib/Target/Hexagon/HexagonGenInsert.cpp
+++ b/llvm/lib/Target/Hexagon/HexagonGenInsert.cpp
@@ -921,6 +921,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".
@@ -943,8 +947,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..a298930b6da6d
--- /dev/null
+++ b/llvm/test/CodeGen/Hexagon/insert-big.ll
@@ -0,0 +1,45 @@
+; 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
+
+; Look for this symptom, in case llc does not check invalid IR.
+; CHECK-NOT: insert(%14,%5,#5,#5)
+
+; RUN: llc -O2 -mtriple=hexagon -insert-max-ifmap=1 -debug-only=hexinsert -stop-after hexinsert < %s 2>&1 | FileCheck %s
+; RUN: llc -O2 -mtriple=hexagon -insert-max-ifmap=2 -debug-only=hexinsert -stop-after hexinsert < %s 2>&1 | FileCheck %s
+; RUN: llc -O2 -mtriple=hexagon -insert-max-ifmap=3 -debug-only=hexinsert -stop-after hexinsert < %s 2>&1 | FileCheck %s
+; RUN: llc -O2 -mtriple=hexagon -insert-max-ifmap=4 -debug-only=hexinsert -stop-after hexinsert < %s 2>&1 | FileCheck %s
+; RUN: llc -O2 -mtriple=hexagon -insert-max-ifmap=5 -debug-only=hexinsert -stop-after hexinsert < %s 2>&1 | FileCheck %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
+}
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Needs db478ba to not break the build.
This test uses -debug-only, so needs an assertion-enabled build.
Backport f3a60cf
Requested by: @androm3da