Skip to content
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

[RISCV] Don't attempt PRE if available info is SEW/LMUL ratio only #77063

Merged
merged 1 commit into from Jan 7, 2024

Conversation

lukel97
Copy link
Contributor

@lukel97 lukel97 commented Jan 5, 2024

We may attempt to do PRE on a block where one predecessor's exit info is
unknown, and one predecssor's exit info has the SEW/LMUL ratio only.

This should be avoided since we can't insert a vsetvli if we don't know its
exact VTYPE (yet).

Normally PRE would bail in these cases because coincidentally it wouldn't have
been profitable since not enough transitions would have been removed. However
after 6707b33, a PseudoVMV_X_S may now be enough to convince PRE that it is
indeed profitable, triggering the assertion when inserting vsetvli.

This fixes the issue described in
#74049 (comment) by
bailing if the available info has the SEW/LMUL ratio only (i.e we don't know
the exact VTYPE)

We may attempt to do PRE on a block where one predecessor's exit info is
unknown, and one predecssor's exit info has the SEW/LMUL ratio only.

This should be avoided since we can't insert a vsetvli if we don't know its
exact VTYPE (yet).

Normally PRE would bail in these cases because coincidentally it wouldn't have
been profitable since not enough transitions would have been removed. However
after 6707b33, a PseudoVMV_X_S may now be enough to convince PRE that it is
indeed profitable, triggering the assertion when inserting vsetvli.

This fixes the issue described in
llvm#74049 (comment) by
bailing if the available info has the SEW/LMUL ratio only (i.e we don't know
the exact VTYPE)
@llvmbot
Copy link
Collaborator

llvmbot commented Jan 5, 2024

@llvm/pr-subscribers-backend-risc-v

Author: Luke Lau (lukel97)

Changes

We may attempt to do PRE on a block where one predecessor's exit info is
unknown, and one predecssor's exit info has the SEW/LMUL ratio only.

This should be avoided since we can't insert a vsetvli if we don't know its
exact VTYPE (yet).

Normally PRE would bail in these cases because coincidentally it wouldn't have
been profitable since not enough transitions would have been removed. However
after 6707b33, a PseudoVMV_X_S may now be enough to convince PRE that it is
indeed profitable, triggering the assertion when inserting vsetvli.

This fixes the issue described in
#74049 (comment) by
bailing if the available info has the SEW/LMUL ratio only (i.e we don't know
the exact VTYPE)


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

2 Files Affected:

  • (modified) llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp (+5)
  • (modified) llvm/test/CodeGen/RISCV/rvv/vsetvli-insert.mir (+57)
diff --git a/llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp b/llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp
index 3400b24e0abb01..e591aa935c0bfc 100644
--- a/llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp
+++ b/llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp
@@ -1381,6 +1381,11 @@ void RISCVInsertVSETVLI::doPRE(MachineBasicBlock &MBB) {
   if (!UnavailablePred || !AvailableInfo.isValid())
     return;
 
+  // If we don't know the exact VTYPE, we can't copy the vsetvli to the exit of
+  // the unavailable pred.
+  if (AvailableInfo.hasSEWLMULRatioOnly())
+    return;
+
   // Critical edge - TODO: consider splitting?
   if (UnavailablePred->succ_size() != 1)
     return;
diff --git a/llvm/test/CodeGen/RISCV/rvv/vsetvli-insert.mir b/llvm/test/CodeGen/RISCV/rvv/vsetvli-insert.mir
index 7bda7a387c68f9..d515022b74183b 100644
--- a/llvm/test/CodeGen/RISCV/rvv/vsetvli-insert.mir
+++ b/llvm/test/CodeGen/RISCV/rvv/vsetvli-insert.mir
@@ -72,6 +72,10 @@
     ret void
   }
 
+  define void @pre_same_sewlmul_ratio() {
+    ret void
+  }
+
   declare <vscale x 1 x i64> @llvm.riscv.vadd.nxv1i64.nxv1i64.i64(<vscale x 1 x i64>, <vscale x 1 x i64>, <vscale x 1 x i64>, i64) #1
 
   declare <vscale x 1 x i64> @llvm.riscv.vle.nxv1i64.i64(<vscale x 1 x i64>, <vscale x 1 x i64>* nocapture, i64) #4
@@ -446,3 +450,56 @@ body:             |
     %4:vr = PseudoVMV_V_I_MF4 undef %4, 0, 4, 3, 0
     PseudoRET
 ...
+---
+# make sure we don't try to perform PRE when one of the blocks is sew/lmul ratio
+# only
+name: pre_same_sewlmul_ratio
+tracksRegLiveness: true
+body:             |
+  ; CHECK-LABEL: name: pre_same_sewlmul_ratio
+  ; CHECK: bb.0:
+  ; CHECK-NEXT:   successors: %bb.2(0x40000000), %bb.1(0x40000000)
+  ; CHECK-NEXT:   liveins: $x10
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT:   %cond:gpr = COPY $x10
+  ; CHECK-NEXT:   dead $x0 = PseudoVSETIVLI 2, 215 /* e32, mf2, ta, ma */, implicit-def $vl, implicit-def $vtype
+  ; CHECK-NEXT:   [[PseudoVMV_V_I_MF2_:%[0-9]+]]:vr = PseudoVMV_V_I_MF2 $noreg, 1, 2, 5 /* e32 */, 0 /* tu, mu */, implicit $vl, implicit $vtype
+  ; CHECK-NEXT:   BEQ %cond, $x0, %bb.2
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT: bb.1:
+  ; CHECK-NEXT:   successors: %bb.2(0x80000000)
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT:   dead $x0 = PseudoVSETVLIX0 killed $x0, 216 /* e64, m1, ta, ma */, implicit-def $vl, implicit-def $vtype, implicit $vl
+  ; CHECK-NEXT:   [[PseudoVMV_V_I_M1_:%[0-9]+]]:vr = PseudoVMV_V_I_M1 $noreg, 1, 2, 6 /* e64 */, 0 /* tu, mu */, implicit $vl, implicit $vtype
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT: bb.2:
+  ; CHECK-NEXT:   successors: %bb.4(0x40000000), %bb.3(0x40000000)
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT:   BEQ %cond, $x0, %bb.4
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT: bb.3:
+  ; CHECK-NEXT:   successors: %bb.4(0x80000000)
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT:   PseudoCALL $noreg, csr_ilp32_lp64
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT: bb.4:
+  ; CHECK-NEXT:   $x0 = PseudoVSETIVLI 2, 215 /* e32, mf2, ta, ma */, implicit-def $vl, implicit-def $vtype
+  ; CHECK-NEXT:   [[PseudoVMV_X_S_MF2_:%[0-9]+]]:gpr = PseudoVMV_X_S_MF2 $noreg, 5 /* e32 */, implicit $vtype
+  ; CHECK-NEXT:   [[PseudoVMV_V_I_MF2_1:%[0-9]+]]:vr = PseudoVMV_V_I_MF2 $noreg, 1, 2, 5 /* e32 */, 0 /* tu, mu */, implicit $vl, implicit $vtype
+  ; CHECK-NEXT:   PseudoRET
+  bb.0:
+    liveins: $x10
+    %cond:gpr = COPY $x10
+    %1:vr = PseudoVMV_V_I_MF2 $noreg, 1, 2, 5, 0
+    BEQ %cond, $x0, %bb.2
+  bb.1:
+    %2:vr = PseudoVMV_V_I_M1 $noreg, 1, 2, 6, 0
+  bb.2: ; the exit info here should have sew/lmul ratio only
+    BEQ %cond, $x0, %bb.4
+  bb.3:
+    PseudoCALL $noreg, csr_ilp32_lp64
+  bb.4: ; this block will have PRE attempted on it
+    %4:gpr = PseudoVMV_X_S_MF2 $noreg, 5
+    %5:vr = PseudoVMV_V_I_MF2 $noreg, 1, 2, 5, 0
+    PseudoRET
+...

Copy link
Collaborator

@topperc topperc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@lukel97 lukel97 merged commit 274f833 into llvm:main Jan 7, 2024
4 of 5 checks passed
justinfargnoli pushed a commit to justinfargnoli/llvm-project that referenced this pull request Jan 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants