Skip to content

VPlan: add missing case for LogicalAnd; fix crash #93553

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

Merged
merged 2 commits into from
Jun 4, 2024

Conversation

artagnon
Copy link
Contributor

@artagnon artagnon commented May 28, 2024

VPTypeAnalysis::inferScalarTypeForRecipe is missing the case for VPInstruction::LogicalAnd, due to which the test vplan-incomplete-cases.ll crashes. Add this missing case, and move the test in vplan-infer-not-or-type.ll to vplan-incomplete-cases.ll, showing correct codegen for trip-counts 2 and 3.

artagnon added 2 commits May 28, 2024 13:57
The test is identical to the one in vplan-infer-not-or-type.ll, except
that the trip count is now 3 instead of 2.
VPTypeAnalysis::inferScalarTypeForRecipe is missing the case for
VPInstruction::LogicalAnd, due to which the test
vplan-incomplete-cases.ll crashes. Add this missing case, and move the
test in vplan-infer-not-or-type.ll to vplan-incomplete-cases.ll, showing
correct codegen for trip-counts 2 and 3.
@llvmbot
Copy link
Member

llvmbot commented May 28, 2024

@llvm/pr-subscribers-llvm-transforms

Author: Ramkumar Ramachandra (artagnon)

Changes

VPTypeAnalysis::inferScalarTypeForRecipe is missing the case for VPInstruction::LogicalAnd, due to which the test vplan-incomplete-cases.ll crashes. Add this missing case, and move the test in vplan-infer-not-or-type.ll to vplan-incomplete-cases.ll, showing correct codegen for trip-counts 2 and 3.

-- 8< --
Based on #93551.


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

3 Files Affected:

  • (modified) llvm/lib/Transforms/Vectorize/VPlanAnalysis.cpp (+2)
  • (added) llvm/test/Transforms/LoopVectorize/vplan-incomplete-cases.ll (+118)
  • (removed) llvm/test/Transforms/LoopVectorize/vplan-infer-not-or-type.ll (-64)
diff --git a/llvm/lib/Transforms/Vectorize/VPlanAnalysis.cpp b/llvm/lib/Transforms/Vectorize/VPlanAnalysis.cpp
index efe8c21874a3a..860f2a33697fe 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanAnalysis.cpp
+++ b/llvm/lib/Transforms/Vectorize/VPlanAnalysis.cpp
@@ -51,6 +51,8 @@ Type *VPTypeAnalysis::inferScalarTypeForRecipe(const VPInstruction *R) {
            "unexpected scalar type inferred for operand");
     return ResTy;
   }
+  case VPInstruction::LogicalAnd:
+    return IntegerType::get(Ctx, 1);
   case VPInstruction::PtrAdd:
     // Return the type based on the pointer argument (i.e. first operand).
     return inferScalarType(R->getOperand(0));
diff --git a/llvm/test/Transforms/LoopVectorize/vplan-incomplete-cases.ll b/llvm/test/Transforms/LoopVectorize/vplan-incomplete-cases.ll
new file mode 100644
index 0000000000000..2007155fe5485
--- /dev/null
+++ b/llvm/test/Transforms/LoopVectorize/vplan-incomplete-cases.ll
@@ -0,0 +1,118 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 5
+; RUN: opt -passes=loop-vectorize -S %s | FileCheck %s
+
+; This test used to crash due to missing Or/Not cases in inferScalarTypeForRecipe.
+define void @vplan_incomplete_cases_tc2(i8 %x, i8 %y) {
+; CHECK-LABEL: define void @vplan_incomplete_cases_tc2(
+; CHECK-SAME: i8 [[X:%.*]], i8 [[Y:%.*]]) {
+; CHECK-NEXT:  [[ENTRY:.*]]:
+; CHECK-NEXT:    br i1 false, label %[[SCALAR_PH:.*]], label %[[VECTOR_PH:.*]]
+; CHECK:       [[VECTOR_PH]]:
+; CHECK-NEXT:    br label %[[VECTOR_BODY:.*]]
+; CHECK:       [[VECTOR_BODY]]:
+; CHECK-NEXT:    [[INDEX:%.*]] = phi i32 [ 0, %[[VECTOR_PH]] ], [ [[INDEX_NEXT:%.*]], %[[VECTOR_BODY]] ]
+; CHECK-NEXT:    [[INDEX_NEXT]] = add nuw i32 [[INDEX]], 2
+; CHECK-NEXT:    br i1 true, label %[[MIDDLE_BLOCK:.*]], label %[[VECTOR_BODY]], !llvm.loop [[LOOP0:![0-9]+]]
+; CHECK:       [[MIDDLE_BLOCK]]:
+; CHECK-NEXT:    br i1 true, label %[[EXIT:.*]], label %[[SCALAR_PH]]
+; CHECK:       [[SCALAR_PH]]:
+; CHECK-NEXT:    [[BC_RESUME_VAL:%.*]] = phi i8 [ 2, %[[MIDDLE_BLOCK]] ], [ 0, %[[ENTRY]] ]
+; CHECK-NEXT:    br label %[[LOOP_HEADER:.*]]
+; CHECK:       [[LOOP_HEADER]]:
+; CHECK-NEXT:    [[IV:%.*]] = phi i8 [ [[IV_NEXT:%.*]], %[[LATCH:.*]] ], [ [[BC_RESUME_VAL]], %[[SCALAR_PH]] ]
+; CHECK-NEXT:    [[AND:%.*]] = and i8 [[X]], [[Y]]
+; CHECK-NEXT:    [[EXTRACT_T:%.*]] = trunc i8 [[AND]] to i1
+; CHECK-NEXT:    br i1 [[EXTRACT_T]], label %[[LATCH]], label %[[INDIRECT_LATCH:.*]]
+; CHECK:       [[INDIRECT_LATCH]]:
+; CHECK-NEXT:    br label %[[LATCH]]
+; CHECK:       [[LATCH]]:
+; CHECK-NEXT:    [[IV_NEXT]] = add i8 [[IV]], 1
+; CHECK-NEXT:    [[ZEXT:%.*]] = zext i8 [[IV]] to i32
+; CHECK-NEXT:    [[CMP:%.*]] = icmp ult i32 [[ZEXT]], 1
+; CHECK-NEXT:    br i1 [[CMP]], label %[[LOOP_HEADER]], label %[[EXIT]], !llvm.loop [[LOOP3:![0-9]+]]
+; CHECK:       [[EXIT]]:
+; CHECK-NEXT:    ret void
+;
+entry:
+  br label %loop.header
+
+loop.header:                                        ; preds = %latch, %entry
+  %iv = phi i8 [ %iv.next, %latch ], [ 0, %entry ]
+  %and = and i8 %x, %y
+  %extract.t = trunc i8 %and to i1
+  br i1 %extract.t, label %latch, label %indirect.latch
+
+indirect.latch:                                     ; preds = %loop.header
+  br label %latch
+
+latch:                                              ; preds = %indirect.latch, loop.header
+  %iv.next = add i8 %iv, 1
+  %zext = zext i8 %iv to i32
+  %cmp = icmp ult i32 %zext, 1
+  br i1 %cmp, label %loop.header, label %exit
+
+exit:                                               ; preds = %latch
+  ret void
+}
+
+; This test used to crash due to missing the LogicalAnd case in inferScalarTypeForRecipe.
+define void @vplan_incomplete_cases_tc3(i8 %x, i8 %y) {
+; CHECK-LABEL: define void @vplan_incomplete_cases_tc3(
+; CHECK-SAME: i8 [[X:%.*]], i8 [[Y:%.*]]) {
+; CHECK-NEXT:  [[ENTRY:.*]]:
+; CHECK-NEXT:    br i1 false, label %[[SCALAR_PH:.*]], label %[[VECTOR_PH:.*]]
+; CHECK:       [[VECTOR_PH]]:
+; CHECK-NEXT:    br label %[[VECTOR_BODY:.*]]
+; CHECK:       [[VECTOR_BODY]]:
+; CHECK-NEXT:    [[INDEX:%.*]] = phi i32 [ 0, %[[VECTOR_PH]] ], [ [[INDEX_NEXT:%.*]], %[[VECTOR_BODY]] ]
+; CHECK-NEXT:    [[INDEX_NEXT]] = add i32 [[INDEX]], 4
+; CHECK-NEXT:    br i1 true, label %[[MIDDLE_BLOCK:.*]], label %[[VECTOR_BODY]], !llvm.loop [[LOOP4:![0-9]+]]
+; CHECK:       [[MIDDLE_BLOCK]]:
+; CHECK-NEXT:    br i1 true, label %[[EXIT:.*]], label %[[SCALAR_PH]]
+; CHECK:       [[SCALAR_PH]]:
+; CHECK-NEXT:    [[BC_RESUME_VAL:%.*]] = phi i8 [ 4, %[[MIDDLE_BLOCK]] ], [ 0, %[[ENTRY]] ]
+; CHECK-NEXT:    br label %[[LOOP_HEADER:.*]]
+; CHECK:       [[LOOP_HEADER]]:
+; CHECK-NEXT:    [[IV:%.*]] = phi i8 [ [[IV_NEXT:%.*]], %[[LATCH:.*]] ], [ [[BC_RESUME_VAL]], %[[SCALAR_PH]] ]
+; CHECK-NEXT:    [[AND:%.*]] = and i8 [[X]], [[Y]]
+; CHECK-NEXT:    [[EXTRACT_T:%.*]] = trunc i8 [[AND]] to i1
+; CHECK-NEXT:    br i1 [[EXTRACT_T]], label %[[LATCH]], label %[[INDIRECT_LATCH:.*]]
+; CHECK:       [[INDIRECT_LATCH]]:
+; CHECK-NEXT:    br label %[[LATCH]]
+; CHECK:       [[LATCH]]:
+; CHECK-NEXT:    [[IV_NEXT]] = add i8 [[IV]], 1
+; CHECK-NEXT:    [[ZEXT:%.*]] = zext i8 [[IV]] to i32
+; CHECK-NEXT:    [[CMP:%.*]] = icmp ult i32 [[ZEXT]], 2
+; CHECK-NEXT:    br i1 [[CMP]], label %[[LOOP_HEADER]], label %[[EXIT]], !llvm.loop [[LOOP5:![0-9]+]]
+; CHECK:       [[EXIT]]:
+; CHECK-NEXT:    ret void
+;
+entry:
+  br label %loop.header
+
+loop.header:                                        ; preds = %latch, %entry
+  %iv = phi i8 [ %iv.next, %latch ], [ 0, %entry ]
+  %and = and i8 %x, %y
+  %extract.t = trunc i8 %and to i1
+  br i1 %extract.t, label %latch, label %indirect.latch
+
+indirect.latch:                                     ; preds = %loop.header
+  br label %latch
+
+latch:                                              ; preds = %indirect.latch, loop.header
+  %iv.next = add i8 %iv, 1
+  %zext = zext i8 %iv to i32
+  %cmp = icmp ult i32 %zext, 2
+  br i1 %cmp, label %loop.header, label %exit
+
+exit:                                               ; preds = %latch
+  ret void
+}
+;.
+; CHECK: [[LOOP0]] = distinct !{[[LOOP0]], [[META1:![0-9]+]], [[META2:![0-9]+]]}
+; CHECK: [[META1]] = !{!"llvm.loop.isvectorized", i32 1}
+; CHECK: [[META2]] = !{!"llvm.loop.unroll.runtime.disable"}
+; CHECK: [[LOOP3]] = distinct !{[[LOOP3]], [[META2]], [[META1]]}
+; CHECK: [[LOOP4]] = distinct !{[[LOOP4]], [[META1]], [[META2]]}
+; CHECK: [[LOOP5]] = distinct !{[[LOOP5]], [[META2]], [[META1]]}
+;.
diff --git a/llvm/test/Transforms/LoopVectorize/vplan-infer-not-or-type.ll b/llvm/test/Transforms/LoopVectorize/vplan-infer-not-or-type.ll
deleted file mode 100644
index 102ef699cb379..0000000000000
--- a/llvm/test/Transforms/LoopVectorize/vplan-infer-not-or-type.ll
+++ /dev/null
@@ -1,64 +0,0 @@
-; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 4
-; RUN: opt < %s -passes=loop-vectorize -S | FileCheck %s
-
-; This test used to crash due to missing Or/Not cases in
-; inferScalarTypeForRecipe.
-
-define void @foo(i8 %arg.0, i8 %arg.1) {
-; CHECK-LABEL: define void @foo(
-; CHECK-SAME: i8 [[ARG_0:%.*]], i8 [[ARG_1:%.*]]) {
-; CHECK-NEXT:  entry:
-; CHECK-NEXT:    br i1 false, label [[SCALAR_PH:%.*]], label [[VECTOR_PH:%.*]]
-; CHECK:       vector.ph:
-; CHECK-NEXT:    br label [[VECTOR_BODY:%.*]]
-; CHECK:       vector.body:
-; CHECK-NEXT:    [[INDEX:%.*]] = phi i32 [ 0, [[VECTOR_PH]] ], [ [[INDEX_NEXT:%.*]], [[VECTOR_BODY]] ]
-; CHECK-NEXT:    [[INDEX_NEXT]] = add nuw i32 [[INDEX]], 2
-; CHECK-NEXT:    br i1 true, label [[MIDDLE_BLOCK:%.*]], label [[VECTOR_BODY]], !llvm.loop [[LOOP0:![0-9]+]]
-; CHECK:       middle.block:
-; CHECK-NEXT:    br i1 true, label [[LOOPEXIT:%.*]], label [[SCALAR_PH]]
-; CHECK:       scalar.ph:
-; CHECK-NEXT:    [[BC_RESUME_VAL:%.*]] = phi i8 [ 2, [[MIDDLE_BLOCK]] ], [ 0, [[ENTRY:%.*]] ]
-; CHECK-NEXT:    br label [[LOOP_HEADER:%.*]]
-; CHECK:       loop.header:
-; CHECK-NEXT:    [[INCREMENTOR:%.*]] = phi i8 [ [[ADD:%.*]], [[LATCH:%.*]] ], [ [[BC_RESUME_VAL]], [[SCALAR_PH]] ]
-; CHECK-NEXT:    [[AND:%.*]] = and i8 [[ARG_0]], [[ARG_1]]
-; CHECK-NEXT:    [[EXTRACT_T:%.*]] = trunc i8 [[AND]] to i1
-; CHECK-NEXT:    br i1 [[EXTRACT_T]], label [[LATCH]], label [[INDIRECT_LATCH:%.*]]
-; CHECK:       indirect.latch:
-; CHECK-NEXT:    br label [[LATCH]]
-; CHECK:       latch:
-; CHECK-NEXT:    [[ADD]] = add i8 [[INCREMENTOR]], 1
-; CHECK-NEXT:    [[CONV:%.*]] = zext i8 [[INCREMENTOR]] to i32
-; CHECK-NEXT:    [[CMP:%.*]] = icmp ult i32 [[CONV]], 1
-; CHECK-NEXT:    br i1 [[CMP]], label [[LOOP_HEADER]], label [[LOOPEXIT]], !llvm.loop [[LOOP3:![0-9]+]]
-; CHECK:       loop.exit:
-; CHECK-NEXT:    ret void
-;
-entry:
-  br label %loop.header
-
-loop.header:                                         ; preds = %latch, %entry
-  %incrementor = phi i8 [ %add, %latch ], [ 0, %entry ]
-  %and = and i8 %arg.0, %arg.1
-  %extract.t = trunc i8 %and to i1
-  br i1 %extract.t, label %latch, label %indirect.latch
-
-indirect.latch:                                       ; preds = %loop.header
-  br label %latch
-
-latch:                               ; preds = %loop.header16, %loop.header
-  %add = add i8 %incrementor, 1
-  %conv = zext i8 %incrementor to i32
-  %cmp = icmp ult i32 %conv, 1
-  br i1 %cmp, label %loop.header, label %loop.exit
-
-loop.exit:
-  ret void
-}
-;.
-; CHECK: [[LOOP0]] = distinct !{[[LOOP0]], [[META1:![0-9]+]], [[META2:![0-9]+]]}
-; CHECK: [[META1]] = !{!"llvm.loop.isvectorized", i32 1}
-; CHECK: [[META2]] = !{!"llvm.loop.unroll.runtime.disable"}
-; CHECK: [[LOOP3]] = distinct !{[[LOOP3]], [[META2]], [[META1]]}
-;.

Copy link
Contributor

@fhahn fhahn left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

As this combines the tests, it is probably worth to just skip #93551 to avoid a bit of churn, as the new test would need to go in new file, just to be combined in this one

@artagnon artagnon changed the title LV/VPlan: add missing case for LogicalAnd; fix crash VPlan: add missing case for LogicalAnd; fix crash Jun 4, 2024
@artagnon artagnon merged commit 59cb55d into llvm:main Jun 4, 2024
8 of 10 checks passed
@artagnon artagnon deleted the vplan-incomplete branch June 4, 2024 07:58
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.

3 participants