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.

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

Jump to bottom

[Loads] Fix crash in isSafeToLoadUnconditionally with scalable accessed type #82650

Merged
merged 2 commits into from
Feb 22, 2024
Merged

[Loads] Fix crash in isSafeToLoadUnconditionally with scalable accessed type #82650

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0162205
[Loads] Fix crash in isSafeToLoadUnconditionally with scalable access…
lukel97 Feb 22, 2024
87e7b03
Use actual pointer instead of null, remove check prefixes
lukel97 Feb 22, 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
6 changes: 3 additions & 3 deletions llvm/lib/Analysis/Loads.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -364,7 +364,7 @@ bool llvm::isSafeToLoadUnconditionally(Value *V, Align Alignment, APInt &Size,

if (Size.getBitWidth() > 64)
return false;
const uint64_t LoadSize = Size.getZExtValue();
const TypeSize LoadSize = TypeSize::getFixed(Size.getZExtValue());

// Otherwise, be a little bit aggressive by scanning the local block where we
// want to check to see if the pointer is already being loaded or stored
Expand Down Expand Up @@ -414,11 +414,11 @@ bool llvm::isSafeToLoadUnconditionally(Value *V, Align Alignment, APInt &Size,

// Handle trivial cases.
if (AccessedPtr == V &&
LoadSize <= DL.getTypeStoreSize(AccessedTy))
TypeSize::isKnownLE(LoadSize, DL.getTypeStoreSize(AccessedTy)))
return true;

if (AreEquivalentAddressValues(AccessedPtr->stripPointerCasts(), V) &&
LoadSize <= DL.getTypeStoreSize(AccessedTy))
TypeSize::isKnownLE(LoadSize, DL.getTypeStoreSize(AccessedTy)))
return true;
}
return false;
Expand Down
22 changes: 22 additions & 0 deletions llvm/test/Transforms/VectorCombine/RISCV/load-widening.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 4
; RUN: opt < %s -passes=vector-combine -S -mtriple=riscv32 -mattr=+v | FileCheck %s -check-prefixes=CHECK,RV32
; RUN: opt < %s -passes=vector-combine -S -mtriple=riscv64 -mattr=+v | FileCheck %s -check-prefixes=CHECK,RV64

define void @fixed_load_scalable_src() {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

VectorCombine::widenSubvectorLoad triggers the crash here when calling isSafeToLoadUnconditionally. Let me know if there's a better place to test for this!

; CHECK-LABEL: define void @fixed_load_scalable_src(
; CHECK-SAME: ) #[[ATTR0:[0-9]+]] {
; CHECK-NEXT: entry:
; CHECK-NEXT: store <vscale x 4 x i16> zeroinitializer, ptr null, align 8
; CHECK-NEXT: [[TMP0:%.*]] = load <4 x i16>, ptr null, align 8
; CHECK-NEXT: [[TMP1:%.*]] = shufflevector <4 x i16> [[TMP0]], <4 x i16> zeroinitializer, <8 x i32> <i32 0, i32 1, i32 2, i32 3, i32 poison, i32 poison, i32 poison, i32 poison>
; CHECK-NEXT: ret void
;
entry:
store <vscale x 4 x i16> zeroinitializer, ptr null
lukel97 marked this conversation as resolved.
Show resolved Hide resolved
%0 = load <4 x i16>, ptr null
%1 = shufflevector <4 x i16> %0, <4 x i16> zeroinitializer, <8 x i32> <i32 0, i32 1, i32 2, i32 3, i32 poison, i32 poison, i32 poison, i32 poison>
ret void
}
;; NOTE: These prefixes are unused and the list is autogenerated. Do not add tests below this line:
; RV32: {{.*}}
; RV64: {{.*}}
Copy link
Contributor

Choose a reason for hiding this comment

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

Drop the RV32/RV64 prefixes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't have them initially but UTC complained about differing CHECK lines?

WARNING: Found conflicting asm under the same prefix: 'CHECK'!

Inspecting the actual check lines it seems to be the same though bar the attributes, which is why I presume it's complaining. Dropped them anyway.

Loading