Skip to content

Commit

Permalink
Revert "[OpenMP][FIX] Restrict more unsound assmptions about threading"
Browse files Browse the repository at this point in the history 
This reverts commit 07c3753.

Reason: This change is dependent on a commit that needs to be rolled
back because it broke the ASan buildbot. See
https://reviews.llvm.org/rGfc21f2d7bae2e0be630470cc7ca9323ed5859892 for
more information.
  • Loading branch information
@hctim
hctim committed Dec 17, 2022
1 parent c8468f1 commit 3b05255
Show file tree
Hide file tree
Showing 7 changed files with 69 additions and 206 deletions.
25 changes: 11 additions & 14 deletions llvm/lib/Transforms/IPO/AttributorAttributes.cpp
View file
Expand Up @@ -1076,23 +1076,25 @@ struct AAPointerInfoImpl
QueryingAA, IRPosition::function(Scope), DepClassTy::OPTIONAL);
const auto *ExecDomainAA = A.lookupAAFor<AAExecutionDomain>(
IRPosition::function(Scope), &QueryingAA, DepClassTy::OPTIONAL);
bool AllInSameNoSyncFn = NoSyncAA.isAssumedNoSync();
const bool NoSync = NoSyncAA.isAssumedNoSync();

// Helper to determine if we need to consider threading, which we cannot
// right now. However, if the function is (assumed) nosync or the thread
// executing all instructions is the main thread only we can ignore
// threading.
auto CanIgnoreThreading = [&](const Instruction &I) -> bool {
if (NoSync)
return true;
if (ExecDomainAA && ExecDomainAA->isExecutedByInitialThreadOnly(I))
return true;
return false;
};

// Helper to determine if the access is executed by the same thread as the
// given instruction, for now it is sufficient to avoid any potential
// threading effects as we cannot deal with them anyway.
auto IsSameThreadAsInst = [&](const Access &Acc) -> bool {
return AllInSameNoSyncFn || CanIgnoreThreading(*Acc.getLocalInst());
// load, for now it is sufficient to avoid any potential threading effects
// as we cannot deal with them anyway.
auto IsSameThreadAsLoad = [&](const Access &Acc) -> bool {
return CanIgnoreThreading(*Acc.getLocalInst());
};

// TODO: Use inter-procedural reachability and dominance.
Expand Down Expand Up @@ -1178,14 +1180,10 @@ struct AAPointerInfoImpl
if (FindInterferingWrites && Dominates)
HasBeenWrittenTo = true;

// Track if all interesting accesses are in the same `nosync` function as
// the given instruction.
AllInSameNoSyncFn &= Acc.getRemoteInst()->getFunction() == &Scope;

// For now we only filter accesses based on CFG reasoning which does not
// work yet if we have threading effects, or the access is complicated.
if (CanUseCFGResoning && Dominates && UseDominanceReasoning &&
IsSameThreadAsInst(Acc))
IsSameThreadAsLoad(Acc))
DominatingWrites.insert(&Acc);

InterferingAccesses.push_back({&Acc, Exact});
Expand All @@ -1198,8 +1196,6 @@ struct AAPointerInfoImpl
// the worst case quadratic as we are looking for another write that will
// hide the effect of this one.
auto CanSkipAccess = [&](const Access &Acc, bool Exact) {
if (!IsSameThreadAsInst(Acc))
return false;
if ((!Acc.isWriteOrAssumption() ||
!AA::isPotentiallyReachable(A, *Acc.getRemoteInst(), I, QueryingAA,
&ExclusionSet, IsLiveInCalleeCB)) &&
Expand All @@ -1210,6 +1206,8 @@ struct AAPointerInfoImpl

if (!DT || !UseDominanceReasoning)
return false;
if (!IsSameThreadAsLoad(Acc))
return false;
if (!DominatingWrites.count(&Acc))
return false;
for (const Access *DomAcc : DominatingWrites) {
Expand All @@ -1229,8 +1227,7 @@ struct AAPointerInfoImpl
// succeeded for all or not.
unsigned NumInterferingAccesses = InterferingAccesses.size();
for (auto &It : InterferingAccesses) {
if (!AllInSameNoSyncFn ||
NumInterferingAccesses > MaxInterferingAccesses ||
if (!CanUseCFGResoning || NumInterferingAccesses > MaxInterferingAccesses ||
!CanSkipAccess(*It.first, It.second)) {
if (!UserCB(*It.first, It.second))
return false;
Expand Down
2 changes: 1 addition & 1 deletion llvm/test/Transforms/Attributor/IPConstantProp/return-argument.ll
View file
@@ -1,5 +1,5 @@
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --function-signature --check-attributes --check-globals
; RUN: opt -aa-pipeline=basic-aa -passes=attributor -attributor-manifest-internal -attributor-max-iterations-verify -attributor-annotate-decl-cs -attributor-max-iterations=11 -S < %s | FileCheck %s --check-prefixes=CHECK,TUNIT
; RUN: opt -aa-pipeline=basic-aa -passes=attributor -attributor-manifest-internal -attributor-max-iterations-verify -attributor-annotate-decl-cs -attributor-max-iterations=5 -S < %s | FileCheck %s --check-prefixes=CHECK,TUNIT
; RUN: opt -aa-pipeline=basic-aa -passes=attributor-cgscc -attributor-manifest-internal -attributor-annotate-decl-cs -S < %s | FileCheck %s --check-prefixes=CHECK,CGSCC

;; This function returns its second argument on all return statements
Expand Down
27 changes: 13 additions & 14 deletions llvm/test/Transforms/Attributor/internal-noalias.ll
View file
Expand Up @@ -7,8 +7,8 @@ define dso_local i32 @visible(i32* noalias %A, i32* noalias %B) #0 {
; TUNIT-LABEL: define {{[^@]+}}@visible
; TUNIT-SAME: (i32* noalias nocapture nofree readonly [[A:%.*]], i32* noalias nocapture nofree readonly align 4 [[B:%.*]]) #[[ATTR0:[0-9]+]] {
; TUNIT-NEXT: entry:
; TUNIT-NEXT: [[CALL1:%.*]] = call i32 @noalias_args(i32* noalias nocapture nofree readonly align 4 [[A]], i32* noalias nocapture nofree readonly align 4 [[B]]) #[[ATTR3:[0-9]+]]
; TUNIT-NEXT: [[CALL2:%.*]] = call i32 @noalias_args_argmem(i32* noalias nocapture nofree readonly align 4 [[A]], i32* noalias nocapture nofree readonly align 4 [[B]]) #[[ATTR3]]
; TUNIT-NEXT: [[CALL1:%.*]] = call i32 @noalias_args(i32* noalias nocapture nofree readonly align 4 [[A]], i32* noalias nocapture nofree readonly align 4 [[B]]) #[[ATTR4:[0-9]+]]
; TUNIT-NEXT: [[CALL2:%.*]] = call i32 @noalias_args_argmem(i32* noalias nocapture nofree readonly align 4 [[A]], i32* noalias nocapture nofree readonly align 4 [[B]]) #[[ATTR4]]
; TUNIT-NEXT: [[ADD:%.*]] = add nsw i32 [[CALL1]], [[CALL2]]
; TUNIT-NEXT: ret i32 [[ADD]]
;
Expand Down Expand Up @@ -36,7 +36,7 @@ define private i32 @noalias_args(i32* %A, i32* %B) #0 {
; TUNIT-NEXT: [[TMP0:%.*]] = load i32, i32* [[A]], align 4
; TUNIT-NEXT: [[TMP1:%.*]] = load i32, i32* [[B]], align 4
; TUNIT-NEXT: [[ADD:%.*]] = add nsw i32 [[TMP0]], [[TMP1]]
; TUNIT-NEXT: [[CALL:%.*]] = call i32 @noalias_args_argmem(i32* nocapture nofree noundef nonnull readonly align 4 dereferenceable(4) [[A]], i32* noalias nocapture nofree noundef nonnull readonly align 4 dereferenceable(4) [[B]]) #[[ATTR3]]
; TUNIT-NEXT: [[CALL:%.*]] = call i32 @noalias_args_argmem(i32* nocapture nofree noundef nonnull readonly align 4 dereferenceable(4) [[A]], i32* noalias nocapture nofree noundef nonnull readonly align 4 dereferenceable(4) [[B]]) #[[ATTR4]]
; TUNIT-NEXT: [[ADD2:%.*]] = add nsw i32 [[ADD]], [[CALL]]
; TUNIT-NEXT: ret i32 [[ADD2]]
;
Expand Down Expand Up @@ -94,8 +94,8 @@ define dso_local i32 @visible_local(i32* %A) #0 {
; TUNIT-NEXT: entry:
; TUNIT-NEXT: [[B:%.*]] = alloca i32, align 4
; TUNIT-NEXT: store i32 5, i32* [[B]], align 4
; TUNIT-NEXT: [[CALL1:%.*]] = call i32 @noalias_args(i32* nocapture nofree readonly align 4 [[A]], i32* noalias nocapture nofree noundef nonnull readonly align 4 dereferenceable(4) [[B]]) #[[ATTR3]]
; TUNIT-NEXT: [[CALL2:%.*]] = call i32 @noalias_args_argmem(i32* nocapture nofree readonly align 4 [[A]], i32* noalias nocapture nofree noundef nonnull readonly align 4 dereferenceable(4) [[B]]) #[[ATTR3]]
; TUNIT-NEXT: [[CALL1:%.*]] = call i32 @noalias_args(i32* nocapture nofree readonly align 4 [[A]], i32* noalias nocapture nofree noundef nonnull readonly align 4 dereferenceable(4) [[B]]) #[[ATTR4]]
; TUNIT-NEXT: [[CALL2:%.*]] = call i32 @noalias_args_argmem(i32* nocapture nofree readonly align 4 [[A]], i32* noalias nocapture nofree noundef nonnull readonly align 4 dereferenceable(4) [[B]]) #[[ATTR4]]
; TUNIT-NEXT: [[ADD:%.*]] = add nsw i32 [[CALL1]], [[CALL2]]
; TUNIT-NEXT: ret i32 [[ADD]]
;
Expand Down Expand Up @@ -158,11 +158,10 @@ define i32 @visible_local_2() {
}

define internal i32 @noalias_args_argmem_rn(i32* %A, i32* %B) #1 {
; TUNIT: Function Attrs: nofree noinline norecurse nosync nounwind willreturn memory(argmem: readwrite) uwtable
; TUNIT: Function Attrs: nofree noinline norecurse nosync nounwind willreturn memory(argmem: write) uwtable
; TUNIT-LABEL: define {{[^@]+}}@noalias_args_argmem_rn
; TUNIT-SAME: (i32* noalias nocapture nofree noundef nonnull align 4 dereferenceable(4) [[B:%.*]]) #[[ATTR1]] {
; TUNIT-NEXT: [[T0:%.*]] = load i32, i32* [[B]], align 4
; TUNIT-NEXT: ret i32 [[T0]]
; TUNIT-SAME: (i32* noalias nocapture nofree noundef nonnull writeonly align 4 dereferenceable(4) [[B:%.*]]) #[[ATTR3:[0-9]+]] {
; TUNIT-NEXT: ret i32 undef
;
; CGSCC: Function Attrs: nofree noinline norecurse nosync nounwind willreturn memory(argmem: readwrite) uwtable
; CGSCC-LABEL: define {{[^@]+}}@noalias_args_argmem_rn
Expand All @@ -181,9 +180,8 @@ define i32 @visible_local_3() {
; TUNIT-LABEL: define {{[^@]+}}@visible_local_3
; TUNIT-SAME: () #[[ATTR2]] {
; TUNIT-NEXT: [[B:%.*]] = alloca i32, align 4
; TUNIT-NEXT: store i32 5, i32* [[B]], align 4
; TUNIT-NEXT: [[CALL:%.*]] = call i32 @noalias_args_argmem_rn(i32* noalias nocapture nofree noundef nonnull align 4 dereferenceable(4) [[B]]) #[[ATTR4:[0-9]+]]
; TUNIT-NEXT: ret i32 [[CALL]]
; TUNIT-NEXT: [[CALL:%.*]] = call i32 @noalias_args_argmem_rn(i32* noalias nocapture nofree noundef nonnull writeonly align 4 dereferenceable(4) [[B]]) #[[ATTR5:[0-9]+]]
; TUNIT-NEXT: ret i32 5
;
; CGSCC: Function Attrs: nofree nosync nounwind willreturn memory(none)
; CGSCC-LABEL: define {{[^@]+}}@visible_local_3
Expand All @@ -205,8 +203,9 @@ attributes #1 = { argmemonly noinline nounwind uwtable willreturn}
; TUNIT: attributes #[[ATTR0]] = { nofree noinline norecurse nosync nounwind willreturn memory(argmem: read) uwtable }
; TUNIT: attributes #[[ATTR1]] = { nofree noinline norecurse nosync nounwind willreturn memory(argmem: readwrite) uwtable }
; TUNIT: attributes #[[ATTR2]] = { nofree norecurse nosync nounwind willreturn memory(none) }
; TUNIT: attributes #[[ATTR3]] = { nofree nosync nounwind }
; TUNIT: attributes #[[ATTR4]] = { nofree nosync nounwind willreturn }
; TUNIT: attributes #[[ATTR3]] = { nofree noinline norecurse nosync nounwind willreturn memory(argmem: write) uwtable }
; TUNIT: attributes #[[ATTR4]] = { nofree nosync nounwind }
; TUNIT: attributes #[[ATTR5]] = { nofree nosync nounwind willreturn }
;.
; CGSCC: attributes #[[ATTR0]] = { nofree noinline nosync nounwind willreturn memory(argmem: read) uwtable }
; CGSCC: attributes #[[ATTR1]] = { nofree noinline norecurse nosync nounwind willreturn memory(argmem: read) uwtable }
Expand Down
24 changes: 3 additions & 21 deletions llvm/test/Transforms/Attributor/value-simplify-assume.ll
View file
@@ -1,5 +1,5 @@
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --function-signature --check-attributes --check-globals
; RUN: opt -aa-pipeline=basic-aa -passes=attributor -attributor-manifest-internal -attributor-max-iterations-verify -attributor-annotate-decl-cs -attributor-max-iterations=3 -S < %s | FileCheck %s --check-prefixes=CHECK,TUNIT
; RUN: opt -aa-pipeline=basic-aa -passes=attributor -attributor-manifest-internal -attributor-max-iterations-verify -attributor-annotate-decl-cs -attributor-max-iterations=4 -S < %s | FileCheck %s --check-prefixes=CHECK,TUNIT
; RUN: opt -aa-pipeline=basic-aa -passes=attributor-cgscc -attributor-manifest-internal -attributor-annotate-decl-cs -S < %s | FileCheck %s --check-prefixes=CHECK,CGSCC

@Gstatic_int1 = internal global i32 zeroinitializer, align 4
Expand Down Expand Up @@ -422,9 +422,6 @@ define i1 @assume_3_nr(i1 %arg, i1 %cond) norecurse {
; TUNIT-LABEL: define {{[^@]+}}@assume_3_nr
; TUNIT-SAME: (i1 [[ARG:%.*]], i1 [[COND:%.*]]) #[[ATTR3]] {
; TUNIT-NEXT: [[STACK:%.*]] = alloca i1, align 1
; TUNIT-NEXT: store i1 [[ARG]], i1* [[STACK]], align 1
; TUNIT-NEXT: [[L:%.*]] = load i1, i1* [[STACK]], align 1
; TUNIT-NEXT: call void @llvm.assume(i1 noundef [[L]]) #[[ATTR6]]
; TUNIT-NEXT: br i1 [[COND]], label [[T:%.*]], label [[F:%.*]]
; TUNIT: t:
; TUNIT-NEXT: store i1 true, i1* [[STACK]], align 1
Expand Down Expand Up @@ -475,7 +472,6 @@ define i1 @assume_4_nr(i1 %arg, i1 %cond) norecurse {
; TUNIT-LABEL: define {{[^@]+}}@assume_4_nr
; TUNIT-SAME: (i1 [[ARG:%.*]], i1 [[COND:%.*]]) #[[ATTR3]] {
; TUNIT-NEXT: [[STACK:%.*]] = alloca i1, align 1
; TUNIT-NEXT: store i1 [[ARG]], i1* [[STACK]], align 1
; TUNIT-NEXT: br i1 [[COND]], label [[T:%.*]], label [[F:%.*]]
; TUNIT: t:
; TUNIT-NEXT: store i1 true, i1* [[STACK]], align 1
Expand Down Expand Up @@ -528,9 +524,6 @@ define i1 @assume_5_nr(i1 %arg, i1 %cond) norecurse {
; TUNIT-LABEL: define {{[^@]+}}@assume_5_nr
; TUNIT-SAME: (i1 [[ARG:%.*]], i1 [[COND:%.*]]) #[[ATTR3]] {
; TUNIT-NEXT: [[STACK:%.*]] = alloca i1, align 1
; TUNIT-NEXT: store i1 [[ARG]], i1* [[STACK]], align 1
; TUNIT-NEXT: [[L1:%.*]] = load i1, i1* [[STACK]], align 1
; TUNIT-NEXT: call void @llvm.assume(i1 noundef [[L1]]) #[[ATTR6]]
; TUNIT-NEXT: br i1 [[COND]], label [[T:%.*]], label [[F:%.*]]
; TUNIT: t:
; TUNIT-NEXT: store i1 true, i1* [[STACK]], align 1
Expand Down Expand Up @@ -599,9 +592,7 @@ define i1 @assume_5c_nr(i1 %cond) norecurse {
; TUNIT-LABEL: define {{[^@]+}}@assume_5c_nr
; TUNIT-SAME: (i1 [[COND:%.*]]) #[[ATTR3]] {
; TUNIT-NEXT: [[STACK:%.*]] = alloca i1, align 1
; TUNIT-NEXT: store i1 true, i1* [[STACK]], align 1
; TUNIT-NEXT: [[L1:%.*]] = load i1, i1* [[STACK]], align 1
; TUNIT-NEXT: call void @llvm.assume(i1 noundef [[L1]]) #[[ATTR6]]
; TUNIT-NEXT: call void @llvm.assume(i1 noundef true) #[[ATTR6]]
; TUNIT-NEXT: br i1 [[COND]], label [[T:%.*]], label [[F:%.*]]
; TUNIT: t:
; TUNIT-NEXT: store i1 true, i1* [[STACK]], align 1
Expand Down Expand Up @@ -1046,9 +1037,6 @@ define i1 @assume_3(i1 %arg, i1 %cond) {
; TUNIT-LABEL: define {{[^@]+}}@assume_3
; TUNIT-SAME: (i1 [[ARG:%.*]], i1 [[COND:%.*]]) #[[ATTR3]] {
; TUNIT-NEXT: [[STACK:%.*]] = alloca i1, align 1
; TUNIT-NEXT: store i1 [[ARG]], i1* [[STACK]], align 1
; TUNIT-NEXT: [[L:%.*]] = load i1, i1* [[STACK]], align 1
; TUNIT-NEXT: call void @llvm.assume(i1 noundef [[L]]) #[[ATTR6]]
; TUNIT-NEXT: br i1 [[COND]], label [[T:%.*]], label [[F:%.*]]
; TUNIT: t:
; TUNIT-NEXT: store i1 true, i1* [[STACK]], align 1
Expand Down Expand Up @@ -1099,7 +1087,6 @@ define i1 @assume_4(i1 %arg, i1 %cond) {
; TUNIT-LABEL: define {{[^@]+}}@assume_4
; TUNIT-SAME: (i1 [[ARG:%.*]], i1 [[COND:%.*]]) #[[ATTR3]] {
; TUNIT-NEXT: [[STACK:%.*]] = alloca i1, align 1
; TUNIT-NEXT: store i1 [[ARG]], i1* [[STACK]], align 1
; TUNIT-NEXT: br i1 [[COND]], label [[T:%.*]], label [[F:%.*]]
; TUNIT: t:
; TUNIT-NEXT: store i1 true, i1* [[STACK]], align 1
Expand Down Expand Up @@ -1152,9 +1139,6 @@ define i1 @assume_5(i1 %arg, i1 %cond) {
; TUNIT-LABEL: define {{[^@]+}}@assume_5
; TUNIT-SAME: (i1 [[ARG:%.*]], i1 [[COND:%.*]]) #[[ATTR3]] {
; TUNIT-NEXT: [[STACK:%.*]] = alloca i1, align 1
; TUNIT-NEXT: store i1 [[ARG]], i1* [[STACK]], align 1
; TUNIT-NEXT: [[L1:%.*]] = load i1, i1* [[STACK]], align 1
; TUNIT-NEXT: call void @llvm.assume(i1 noundef [[L1]]) #[[ATTR6]]
; TUNIT-NEXT: br i1 [[COND]], label [[T:%.*]], label [[F:%.*]]
; TUNIT: t:
; TUNIT-NEXT: store i1 true, i1* [[STACK]], align 1
Expand Down Expand Up @@ -1223,9 +1207,7 @@ define i1 @assume_5c(i1 %cond) {
; TUNIT-LABEL: define {{[^@]+}}@assume_5c
; TUNIT-SAME: (i1 [[COND:%.*]]) #[[ATTR3]] {
; TUNIT-NEXT: [[STACK:%.*]] = alloca i1, align 1
; TUNIT-NEXT: store i1 true, i1* [[STACK]], align 1
; TUNIT-NEXT: [[L1:%.*]] = load i1, i1* [[STACK]], align 1
; TUNIT-NEXT: call void @llvm.assume(i1 noundef [[L1]]) #[[ATTR6]]
; TUNIT-NEXT: call void @llvm.assume(i1 noundef true) #[[ATTR6]]
; TUNIT-NEXT: br i1 [[COND]], label [[T:%.*]], label [[F:%.*]]
; TUNIT: t:
; TUNIT-NEXT: store i1 true, i1* [[STACK]], align 1
Expand Down

0 comments on commit 3b05255

Please sign in to comment.