Skip to content

[RISCV] Fix crash when unrolling loop containing vector instructions #83384

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.

Sign up for GitHub

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 4 commits into from
Mar 2, 2024
Merged

[RISCV] Fix crash when unrolling loop containing vector instructions #83384

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a8900f9
[RISCV] Fix crash when unrolling loop containing vector instructions
arcbbb Feb 29, 2024
736edf9
Rename test
arcbbb Feb 29, 2024
4ec95de
Fix typo
arcbbb Feb 29, 2024
f4e6633
rename the test to vector-cost-without-v.ll
arcbbb Mar 1, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions llvm/lib/Target/RISCV/RISCVTargetTransformInfo.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,9 @@ static cl::opt<unsigned> SLPMaxVF(
InstructionCost
RISCVTTIImpl::getRISCVInstructionCost(ArrayRef<unsigned> OpCodes, MVT VT,
Copy link
Contributor

Choose a reason for hiding this comment

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

This might be an aside but I find it a little surprising that this function's called getRISCVInstructionCost but it only handles vector instructions. Sounds like it should be getRVVInstructionCost. I might be missing something though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point. Your suggested name appears to be clearer.

TTI::TargetCostKind CostKind) {
// Check if the type is valid for all CostKind
if (!VT.isVector())
return InstructionCost::getInvalid();
Copy link
Contributor

Choose a reason for hiding this comment

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

Where is this getting called from with a scalar MVT? I would have thought that after scalarization we wouldn't have any vector instructions that would still be calling into this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is from LoopUnrollPass along the way from TTI.getInstructionCost to TTI.getShuffleCost for

%splat.splat.i.i.i = shufflevector <2 x float> zeroinitializer, <2 x float> zeroinitializer, <2 x i32> zeroinitializer

The data type is <2 x float>, but becomes float after getTypeLegalizationCost(Tp); then enters getRISCVInstructionCost().
I'm just not sure how a vector type can be generated without a vector support.

If it is true, we might consider checking if the type is still vector after legalization at the beginning of TTI functions which accept vector types.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah ok, thanks for clarifying. I guess there's nothing stopping a frontend from just emitting vector LLVM IR independently of the autovectorizers.

I'm not strongly opinionated about where we check that the legalized type is vector, I presume both will result in an invalid cost

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, in our case it's because we're compiling OpenCL or SYCL, with vector data types in the source.

size_t NumInstr = OpCodes.size();
if (CostKind == TTI::TCK_CodeSize)
return NumInstr;
Expand Down
53 changes: 53 additions & 0 deletions llvm/test/Analysis/CostModel/RISCV/vector-cost-without-v.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 4
; RUN: opt < %s -mtriple=riscv64 -mattr=+f,+d --passes=loop-unroll-full -S | FileCheck %s

; Check it doesn't crash when the vector extension is not enabled.
define void @foo() {
; CHECK-LABEL: define void @foo(
; CHECK-SAME: ) #[[ATTR0:[0-9]+]] {
; CHECK-NEXT: entry:
; CHECK-NEXT: br label [[FOR_BODY:%.*]]
; CHECK: for.body:
; CHECK-NEXT: [[INDVARS_IV1:%.*]] = phi i64 [ [[INDVARS_IV_NEXT:%.*]], [[FOR_BODY]] ], [ 0, [[ENTRY:%.*]] ]
; CHECK-NEXT: [[TMP0:%.*]] = load float, ptr null, align 4
; CHECK-NEXT: [[SPLAT_SPLAT_I_I_I:%.*]] = shufflevector <2 x float> zeroinitializer, <2 x float> zeroinitializer, <2 x i32> zeroinitializer
; CHECK-NEXT: [[CMP1_I_I_I:%.*]] = fcmp ogt <2 x float> zeroinitializer, zeroinitializer
; CHECK-NEXT: [[SPLAT_SPLAT3_I_I_I:%.*]] = shufflevector <2 x i32> zeroinitializer, <2 x i32> zeroinitializer, <2 x i32> zeroinitializer
; CHECK-NEXT: [[XOR3_I_I_I_I_I:%.*]] = select <2 x i1> zeroinitializer, <2 x i32> zeroinitializer, <2 x i32> zeroinitializer
; CHECK-NEXT: [[TMP1:%.*]] = load float, ptr null, align 4
; CHECK-NEXT: [[SPLAT_SPLAT8_I_I_I:%.*]] = shufflevector <2 x float> zeroinitializer, <2 x float> zeroinitializer, <2 x i32> zeroinitializer
; CHECK-NEXT: [[SUB_I_I_I:%.*]] = fsub <2 x float> zeroinitializer, zeroinitializer
; CHECK-NEXT: [[MUL_I_I_I:%.*]] = shl i64 0, 0
; CHECK-NEXT: [[TMP2:%.*]] = load float, ptr null, align 4
; CHECK-NEXT: [[SPLAT_SPLAT_I_I_I_I:%.*]] = shufflevector <2 x float> zeroinitializer, <2 x float> zeroinitializer, <2 x i32> zeroinitializer
; CHECK-NEXT: [[XOR3_I_I_I_V_I_I:%.*]] = select <2 x i1> zeroinitializer, <2 x float> zeroinitializer, <2 x float> zeroinitializer
; CHECK-NEXT: [[INDVARS_IV_NEXT]] = add i64 [[INDVARS_IV1]], 1
; CHECK-NEXT: [[EXITCOND:%.*]] = icmp ne i64 [[INDVARS_IV1]], 8
; CHECK-NEXT: br i1 [[EXITCOND]], label [[FOR_BODY]], label [[EXIT:%.*]]
; CHECK: exit:
; CHECK-NEXT: ret void
;
entry:
br label %for.body

for.body: ; preds = %for.body, %entry
%indvars.iv1 = phi i64 [ %indvars.iv.next, %for.body ], [ 0, %entry ]
%0 = load float, ptr null, align 4
%splat.splat.i.i.i = shufflevector <2 x float> zeroinitializer, <2 x float> zeroinitializer, <2 x i32> zeroinitializer
%cmp1.i.i.i = fcmp ogt <2 x float> zeroinitializer, zeroinitializer
%splat.splat3.i.i.i = shufflevector <2 x i32> zeroinitializer, <2 x i32> zeroinitializer, <2 x i32> zeroinitializer
%xor3.i.i.i.i.i = select <2 x i1> zeroinitializer, <2 x i32> zeroinitializer, <2 x i32> zeroinitializer
%1 = load float, ptr null, align 4
%splat.splat8.i.i.i = shufflevector <2 x float> zeroinitializer, <2 x float> zeroinitializer, <2 x i32> zeroinitializer
%sub.i.i.i = fsub <2 x float> zeroinitializer, zeroinitializer
%mul.i.i.i = shl i64 0, 0
%2 = load float, ptr null, align 4
%splat.splat.i.i.i.i = shufflevector <2 x float> zeroinitializer, <2 x float> zeroinitializer, <2 x i32> zeroinitializer
%xor3.i.i.i.v.i.i = select <2 x i1> zeroinitializer, <2 x float> zeroinitializer, <2 x float> zeroinitializer
%indvars.iv.next = add i64 %indvars.iv1, 1
%exitcond = icmp ne i64 %indvars.iv1, 8
br i1 %exitcond, label %for.body, label %exit

exit: ; preds = %for.body
ret void
}