[llvm] [GlobalISel] Don't push `freeze`s post-legalizer when that creates illegal instruction (PR #204785)

[via llvm-commits]

=?utf-8?q?Ömer_Sinan_Ağacan?= <omeragacan at gmail.com>
Message-ID:
In-Reply-To: <llvm.org/llvm/llvm-project/pull/204785 at github.com>


llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-backend-aarch64

@llvm/pr-subscribers-llvm-globalisel

Author: Ömer Sinan Ağacan (osa1)

<details>
<summary>Changes</summary>

In the combine rule

    // Convert freeze(Op(Op0, NonPoisonOps...)) to Op(freeze(Op0), NonPoisonOps...)
    // when Op0 is not guaranteed non-poison
    def push_freeze_to_prevent_poison_from_propagating ...

Don't push the `freeze` if `freeze(Op0)` is illegal and we've already
run the legalizer.

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


2 Files Affected:

- (modified) llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp (+6) 
- (added) llvm/test/CodeGen/AArch64/GlobalISel/freeze-push.ll (+73) 


``````````diff
diff --git a/llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp b/llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp
index 8853a941e137f..217074d54f46c 100644
--- a/llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp
+++ b/llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp
@@ -252,6 +252,8 @@ void CombinerHelper::applyCombineCopy(MachineInstr &MI) const {
 
 bool CombinerHelper::matchFreezeOfSingleMaybePoisonOperand(
     MachineInstr &MI, BuildFnTy &MatchInfo) const {
+  assert(MI.getOpcode() == TargetOpcode::G_FREEZE && "Invalid instruction");
+
   // Ported from InstCombinerImpl::pushFreezeToPreventPoisonFromPropagating.
   Register DstOp = MI.getOperand(0).getReg();
   Register OrigOp = MI.getOperand(1).getReg();
@@ -306,6 +308,10 @@ bool CombinerHelper::matchFreezeOfSingleMaybePoisonOperand(
   Register MaybePoisonOperandReg = MaybePoisonOperand->getReg();
   LLT MaybePoisonOperandRegTy = MRI.getType(MaybePoisonOperandReg);
 
+  if (!isLegalOrBeforeLegalizer(
+          {TargetOpcode::G_FREEZE, {MaybePoisonOperandRegTy}}))
+    return false;
+
   MatchInfo = [=](MachineIRBuilder &B) mutable {
     Observer.changingInstr(*OrigDef);
     cast<GenericMachineInstr>(OrigDef)->dropPoisonGeneratingFlags();
diff --git a/llvm/test/CodeGen/AArch64/GlobalISel/freeze-push.ll b/llvm/test/CodeGen/AArch64/GlobalISel/freeze-push.ll
new file mode 100644
index 0000000000000..dd6e6d172ed5e
--- /dev/null
+++ b/llvm/test/CodeGen/AArch64/GlobalISel/freeze-push.ll
@@ -0,0 +1,73 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -mtriple=arm64-apple-macosx -global-isel < %s | FileCheck %s
+
+; In the combine rule
+;
+;    // Convert freeze(Op(Op0, NonPoisonOps...)) to Op(freeze(Op0), NonPoisonOps...)
+;    // when Op0 is not guaranteed non-poison
+;    def push_freeze_to_prevent_poison_from_propagating ...
+;
+; Don't push the `freeze` if `freeze(Op0)` is illegal and we've already run
+; the legalizer.
+;
+; On AArch64, in the functions below, `freeze`s would be pushed all the way to
+; the `load`, which would then cause the crash:
+;
+;    LLVM ERROR: instruction is not legal: %14:_(i128) = G_FREEZE %0:_ (in function: _0)
+
+define <4 x i32> @_0() {
+; CHECK-LABEL: _0:
+; CHECK:       ; %bb.0: ; %entry
+; CHECK-NEXT:    mov x8, xzr
+; CHECK-NEXT:    ldr q0, [x8]
+; CHECK-NEXT:    fmov x8, d0
+; CHECK-NEXT:    fmov s0, w8
+; CHECK-NEXT:    mov.s v0[1], wzr
+; CHECK-NEXT:    mov.s v0[2], wzr
+; CHECK-NEXT:    mov.s v0[3], wzr
+; CHECK-NEXT:    ret
+entry:
+  %0 = load i128, ptr null, align 16
+  %1 = freeze i128 %0
+  %2 = insertelement <4 x i128> zeroinitializer, i128 %1, i64 0
+  %3 = trunc <4 x i128> %2 to <4 x i32>
+  ret <4 x i32> %3
+}
+
+define <4 x i32> @_1() {
+; CHECK-LABEL: _1:
+; CHECK:       ; %bb.0: ; %entry
+; CHECK-NEXT:    mov x8, xzr
+; CHECK-NEXT:    ldr q0, [x8]
+; CHECK-NEXT:    fmov x8, d0
+; CHECK-NEXT:    fmov s0, w8
+; CHECK-NEXT:    mov.s v0[1], wzr
+; CHECK-NEXT:    mov.s v0[2], wzr
+; CHECK-NEXT:    mov.s v0[3], wzr
+; CHECK-NEXT:    ret
+entry:
+  %0 = load i128, ptr null, align 16
+  %1 = insertelement <4 x i128> zeroinitializer, i128 %0, i64 0
+  %2 = freeze <4 x i128> %1
+  %3 = trunc <4 x i128> %2 to <4 x i32>
+  ret <4 x i32> %3
+}
+
+define <4 x i32> @_2() {
+; CHECK-LABEL: _2:
+; CHECK:       ; %bb.0: ; %entry
+; CHECK-NEXT:    mov x8, xzr
+; CHECK-NEXT:    ldr q0, [x8]
+; CHECK-NEXT:    fmov x8, d0
+; CHECK-NEXT:    fmov s0, w8
+; CHECK-NEXT:    mov.s v0[1], wzr
+; CHECK-NEXT:    mov.s v0[2], wzr
+; CHECK-NEXT:    mov.s v0[3], wzr
+; CHECK-NEXT:    ret
+entry:
+  %0 = load i128, ptr null, align 16
+  %1 = insertelement <4 x i128> zeroinitializer, i128 %0, i64 0
+  %2 = trunc <4 x i128> %1 to <4 x i32>
+  %3 = freeze <4 x i32> %2
+  ret <4 x i32> %3
+}

``````````

</details>


https://github.com/llvm/llvm-project/pull/204785