Skip to content

Commit

Permalink
[Attributor][FIX] Ensure we adjust types properly
Browse files Browse the repository at this point in the history 
When we simplify loads we need to adjust types (esp. null-values)
properly to avoid inconsinstencies down the line. Add a cast and an
error message.

Fixes: #60788
  • Loading branch information
@jdoerfert @tru
jdoerfert authored and tru committed Feb 21, 2023
1 parent 7eac876 commit 3c27086
Show file tree
Hide file tree
Showing 3 changed files with 147 additions and 4 deletions.
22 changes: 20 additions & 2 deletions llvm/lib/Transforms/IPO/Attributor.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -396,6 +396,18 @@ static bool getPotentialCopiesOfMemoryValue(
NullOnly = false;
};

auto AdjustWrittenValueType = [&](const AAPointerInfo::Access &Acc,
Value &V) {
Value *AdjV = AA::getWithType(V, *I.getType());
if (!AdjV) {
LLVM_DEBUG(dbgs() << "Underlying object written but stored value "
"cannot be converted to read type: "
<< *Acc.getRemoteInst() << " : " << *I.getType()
<< "\n";);
}
return AdjV;
};

auto CheckAccess = [&](const AAPointerInfo::Access &Acc, bool IsExact) {
if ((IsLoad && !Acc.isWriteOrAssumption()) || (!IsLoad && !Acc.isRead()))
return true;
Expand All @@ -417,7 +429,10 @@ static bool getPotentialCopiesOfMemoryValue(
if (IsLoad) {
assert(isa<LoadInst>(I) && "Expected load or store instruction only!");
if (!Acc.isWrittenValueUnknown()) {
NewCopies.push_back(Acc.getWrittenValue());
Value *V = AdjustWrittenValueType(Acc, *Acc.getWrittenValue());
if (!V)
return false;
NewCopies.push_back(V);
NewCopyOrigins.push_back(Acc.getRemoteInst());
return true;
}
Expand All @@ -428,7 +443,10 @@ static bool getPotentialCopiesOfMemoryValue(
<< *Acc.getRemoteInst() << "\n";);
return false;
}
NewCopies.push_back(SI->getValueOperand());
Value *V = AdjustWrittenValueType(Acc, *SI->getValueOperand());
if (!V)
return false;
NewCopies.push_back(V);
NewCopyOrigins.push_back(SI);
} else {
assert(isa<StoreInst>(I) && "Expected load or store instruction only!");
Expand Down
4 changes: 2 additions & 2 deletions llvm/test/Transforms/Attributor/IPConstantProp/openmp_parallel_for.ll
View file
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ define internal void @.omp_outlined.(ptr noalias %.global_tid., ptr noalias %.bo
; TUNIT: omp.inner.for.body:
; TUNIT-NEXT: [[ADD10:%.*]] = add nsw i32 [[DOTOMP_IV_0]], 2
; TUNIT-NEXT: [[TMP11:%.*]] = load double, ptr [[Q_ADDR]], align 8
; TUNIT-NEXT: call void @bar(i32 [[ADD10]], float 3.000000e+00, double noundef [[TMP11]])
; TUNIT-NEXT: call void @bar(i32 [[ADD10]], float 3.000000e+00, double [[TMP11]])
; TUNIT-NEXT: br label [[OMP_BODY_CONTINUE:%.*]]
; TUNIT: omp.body.continue:
; TUNIT-NEXT: br label [[OMP_INNER_FOR_INC]]
Expand Down Expand Up @@ -161,7 +161,7 @@ define internal void @.omp_outlined.(ptr noalias %.global_tid., ptr noalias %.bo
; CGSCC-NEXT: [[ADD10:%.*]] = add nsw i32 [[DOTOMP_IV_0]], 2
; CGSCC-NEXT: [[TMP10:%.*]] = load float, ptr [[P]], align 4
; CGSCC-NEXT: [[TMP11:%.*]] = load double, ptr [[Q_ADDR]], align 8
; CGSCC-NEXT: call void @bar(i32 [[ADD10]], float [[TMP10]], double noundef [[TMP11]])
; CGSCC-NEXT: call void @bar(i32 [[ADD10]], float [[TMP10]], double [[TMP11]])
; CGSCC-NEXT: br label [[OMP_BODY_CONTINUE:%.*]]
; CGSCC: omp.body.continue:
; CGSCC-NEXT: br label [[OMP_INNER_FOR_INC]]
Expand Down
125 changes: 125 additions & 0 deletions llvm/test/Transforms/Attributor/reduced/openmp_opt_constant_type_crash.ll
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,125 @@
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --function-signature --scrub-attributes --check-attributes --check-globals --include-generated-funcs
; RUN: opt -passes=openmp-opt -S < %s | FileCheck %s --check-prefixes=CHECK

target datalayout = "e-p:64:64-p1:64:64-p2:32:32-p3:32:32-p4:64:64-p5:32:32-p6:32:32-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128-v192:256-v256:256-v512:512-v1024:1024-v2048:2048-n32:64-S32-A5-G1-ni:7"
target triple = "amdgcn-amd-amdhsa"

%"struct.Kokkos::Impl::SubviewExtents.448" = type <{ [2 x i64], [1 x i64], [1 x i32], [4 x i8] }>

define linkonce_odr void @_ZN6Kokkos4ViewIPdJNS_12LayoutStrideENS_6DeviceINS_12Experimental12OpenMPTargetENS_18ScratchMemorySpaceIS5_EEEENS_12MemoryTraitsILj1EEEEEC2IPS1_JNS_11LayoutRightES7_SA_ENS_4Impl5ALL_tEJiEEERKNS0_IT_JDpT0_EEET1_DpT2_() {
entry:
call void @_ZN6Kokkos4Impl11ViewMappingIvJNS_10ViewTraitsIPPdJNS_11LayoutRightENS_18ScratchMemorySpaceINS_12Experimental12OpenMPTargetEEENS_12MemoryTraitsILj1EEEEEENS0_5ALL_tEiEE6assignINS2_IS3_JNS_12LayoutStrideENS_6DeviceIS8_S9_EESB_EEEEEvRNS1_IT_JvEEERKNS1_ISC_JvEEESD_i()
ret void
}

define linkonce_odr void @_ZN6Kokkos4Impl11ViewMappingIvJNS_10ViewTraitsIPPdJNS_11LayoutRightENS_18ScratchMemorySpaceINS_12Experimental12OpenMPTargetEEENS_12MemoryTraitsILj1EEEEEENS0_5ALL_tEiEE6assignINS2_IS3_JNS_12LayoutStrideENS_6DeviceIS8_S9_EESB_EEEEEvRNS1_IT_JvEEERKNS1_ISC_JvEEESD_i() {
entry:
%extents11 = alloca [0 x [0 x %"struct.Kokkos::Impl::SubviewExtents.448"]], i32 0, align 8, addrspace(5)
%extents.ascast = addrspacecast ptr addrspace(5) %extents11 to ptr
call void @_ZN6Kokkos4Impl14SubviewExtentsILj2ELj1EEC2IJLm0ELm0EEJNS0_5ALL_tEiEEERKNS0_13ViewDimensionIJXspT_EEEEDpT0_(ptr %extents.ascast)
call void @_ZN6Kokkos4Impl10ViewOffsetINS0_13ViewDimensionIJLm0EEEENS_12LayoutStrideEvEC2INS2_IJLm0ELm0EEEENS_11LayoutRightEEERKNS1_IT_T0_vEERKNS0_14SubviewExtentsIXsrS9_4rankELj1EEE(ptr %extents.ascast)
ret void
}

define linkonce_odr void @_ZN6Kokkos4Impl14SubviewExtentsILj2ELj1EEC2IJLm0ELm0EEJNS0_5ALL_tEiEEERKNS0_13ViewDimensionIJXspT_EEEEDpT0_(ptr %this) {
entry:
%call = call i1 @_ZN6Kokkos4Impl14SubviewExtentsILj2ELj1EE3setIJLm0ELm0EEJiEEEbjjRKNS0_13ViewDimensionIJXspT_EEEENS0_5ALL_tEDpT0_(ptr %this)
ret void
}

define linkonce_odr void @_ZN6Kokkos4Impl10ViewOffsetINS0_13ViewDimensionIJLm0EEEENS_12LayoutStrideEvEC2INS2_IJLm0ELm0EEEENS_11LayoutRightEEERKNS1_IT_T0_vEERKNS0_14SubviewExtentsIXsrS9_4rankELj1EEE(ptr %sub) {
entry:
%call191 = call i32 @_ZNK6Kokkos4Impl14SubviewExtentsILj2ELj1EE11range_indexIiEEjT_(ptr %sub)
%call201 = call i64 @_ZN6Kokkos4Impl10ViewOffsetINS0_13ViewDimensionIJLm0EEEENS_12LayoutStrideEvE6strideINS2_IJLm0ELm0EEEENS_11LayoutRightEEEmjRKNS1_IT_T0_vEE(i32 %call191)
ret void
}

define linkonce_odr i1 @_ZN6Kokkos4Impl14SubviewExtentsILj2ELj1EE3setIJLm0ELm0EEJiEEEbjjRKNS0_13ViewDimensionIJXspT_EEEENS0_5ALL_tEDpT0_(ptr %this) {
entry:
store i64 0, ptr %this, align 8
ret i1 false
}

define linkonce_odr i64 @_ZN6Kokkos4Impl10ViewOffsetINS0_13ViewDimensionIJLm0EEEENS_12LayoutStrideEvE6strideINS2_IJLm0ELm0EEEENS_11LayoutRightEEEmjRKNS1_IT_T0_vEE(i32 %r) {
entry:
store i32 %r, ptr null, align 4294967296
ret i64 0
}

define linkonce_odr i32 @_ZNK6Kokkos4Impl14SubviewExtentsILj2ELj1EE11range_indexIiEEjT_(ptr %this) {
entry:
%cmp = icmp eq i32 1, 0
br i1 %cmp, label %cond.true, label %cond.end

cond.true: ; preds = %entry
%0 = load i32, ptr %this, align 4
br label %cond.end

cond.end: ; preds = %cond.true, %entry
%cond = phi i32 [ %0, %cond.true ], [ 1, %entry ]
ret i32 %cond
}

!llvm.module.flags = !{!0, !1}

!0 = !{i32 7, !"openmp", i32 50}
!1 = !{i32 7, !"openmp-device", i32 50}
; CHECK-LABEL: define {{[^@]+}}@_ZN6Kokkos4ViewIPdJNS_12LayoutStrideENS_6DeviceINS_12Experimental12OpenMPTargetENS_18ScratchMemorySpaceIS5_EEEENS_12MemoryTraitsILj1EEEEEC2IPS1_JNS_11LayoutRightES7_SA_ENS_4Impl5ALL_tEJiEEERKNS0_IT_JDpT0_EEET1_DpT2_() {
; CHECK-NEXT: entry:
; CHECK-NEXT: ret void
;
;
; CHECK-LABEL: define {{[^@]+}}@_ZN6Kokkos4Impl11ViewMappingIvJNS_10ViewTraitsIPPdJNS_11LayoutRightENS_18ScratchMemorySpaceINS_12Experimental12OpenMPTargetEEENS_12MemoryTraitsILj1EEEEEENS0_5ALL_tEiEE6assignINS2_IS3_JNS_12LayoutStrideENS_6DeviceIS8_S9_EESB_EEEEEvRNS1_IT_JvEEERKNS1_ISC_JvEEESD_i() {
; CHECK-NEXT: entry:
; CHECK-NEXT: [[EXTENTS11:%.*]] = alloca [0 x [0 x %"struct.Kokkos::Impl::SubviewExtents.448"]], i32 0, align 8, addrspace(5)
; CHECK-NEXT: [[EXTENTS_ASCAST:%.*]] = addrspacecast ptr addrspace(5) [[EXTENTS11]] to ptr
; CHECK-NEXT: call void @_ZN6Kokkos4Impl14SubviewExtentsILj2ELj1EEC2IJLm0ELm0EEJNS0_5ALL_tEiEEERKNS0_13ViewDimensionIJXspT_EEEEDpT0_(ptr [[EXTENTS_ASCAST]])
; CHECK-NEXT: call void @_ZN6Kokkos4Impl10ViewOffsetINS0_13ViewDimensionIJLm0EEEENS_12LayoutStrideEvEC2INS2_IJLm0ELm0EEEENS_11LayoutRightEEERKNS1_IT_T0_vEERKNS0_14SubviewExtentsIXsrS9_4rankELj1EEE(ptr [[EXTENTS_ASCAST]])
; CHECK-NEXT: ret void
;
;
; CHECK-LABEL: define {{[^@]+}}@_ZN6Kokkos4Impl14SubviewExtentsILj2ELj1EEC2IJLm0ELm0EEJNS0_5ALL_tEiEEERKNS0_13ViewDimensionIJXspT_EEEEDpT0_
; CHECK-SAME: (ptr [[THIS:%.*]]) {
; CHECK-NEXT: entry:
; CHECK-NEXT: [[CALL:%.*]] = call i1 @_ZN6Kokkos4Impl14SubviewExtentsILj2ELj1EE3setIJLm0ELm0EEJiEEEbjjRKNS0_13ViewDimensionIJXspT_EEEENS0_5ALL_tEDpT0_(ptr [[THIS]])
; CHECK-NEXT: ret void
;
;
; CHECK-LABEL: define {{[^@]+}}@_ZN6Kokkos4Impl10ViewOffsetINS0_13ViewDimensionIJLm0EEEENS_12LayoutStrideEvEC2INS2_IJLm0ELm0EEEENS_11LayoutRightEEERKNS1_IT_T0_vEERKNS0_14SubviewExtentsIXsrS9_4rankELj1EEE
; CHECK-SAME: (ptr [[SUB:%.*]]) {
; CHECK-NEXT: entry:
; CHECK-NEXT: [[CALL191:%.*]] = call i32 @_ZNK6Kokkos4Impl14SubviewExtentsILj2ELj1EE11range_indexIiEEjT_(ptr [[SUB]])
; CHECK-NEXT: [[CALL201:%.*]] = call i64 @_ZN6Kokkos4Impl10ViewOffsetINS0_13ViewDimensionIJLm0EEEENS_12LayoutStrideEvE6strideINS2_IJLm0ELm0EEEENS_11LayoutRightEEEmjRKNS1_IT_T0_vEE(i32 [[CALL191]])
; CHECK-NEXT: ret void
;
;
; CHECK-LABEL: define {{[^@]+}}@_ZN6Kokkos4Impl14SubviewExtentsILj2ELj1EE3setIJLm0ELm0EEJiEEEbjjRKNS0_13ViewDimensionIJXspT_EEEENS0_5ALL_tEDpT0_
; CHECK-SAME: (ptr [[THIS:%.*]]) {
; CHECK-NEXT: entry:
; CHECK-NEXT: store i64 0, ptr [[THIS]], align 8
; CHECK-NEXT: ret i1 false
;
;
; CHECK-LABEL: define {{[^@]+}}@_ZN6Kokkos4Impl10ViewOffsetINS0_13ViewDimensionIJLm0EEEENS_12LayoutStrideEvE6strideINS2_IJLm0ELm0EEEENS_11LayoutRightEEEmjRKNS1_IT_T0_vEE
; CHECK-SAME: (i32 [[R:%.*]]) {
; CHECK-NEXT: entry:
; CHECK-NEXT: store i32 [[R]], ptr null, align 4294967296
; CHECK-NEXT: ret i64 0
;
;
; CHECK-LABEL: define {{[^@]+}}@_ZNK6Kokkos4Impl14SubviewExtentsILj2ELj1EE11range_indexIiEEjT_
; CHECK-SAME: (ptr [[THIS:%.*]]) {
; CHECK-NEXT: entry:
; CHECK-NEXT: [[CMP:%.*]] = icmp eq i32 1, 0
; CHECK-NEXT: br i1 [[CMP]], label [[COND_TRUE:%.*]], label [[COND_END:%.*]]
; CHECK: cond.true:
; CHECK-NEXT: [[TMP0:%.*]] = load i32, ptr [[THIS]], align 4
; CHECK-NEXT: br label [[COND_END]]
; CHECK: cond.end:
; CHECK-NEXT: [[COND:%.*]] = phi i32 [ [[TMP0]], [[COND_TRUE]] ], [ 1, [[ENTRY:%.*]] ]
; CHECK-NEXT: ret i32 [[COND]]
;
;.
; CHECK: [[META0:![0-9]+]] = !{i32 7, !"openmp", i32 50}
; CHECK: [[META1:![0-9]+]] = !{i32 7, !"openmp-device", i32 50}
;.

0 comments on commit 3c27086

Please sign in to comment.