Skip to content

Commit

Permalink
[Attributor][NFCI] Merge MemoryEffects explicitly
Browse files Browse the repository at this point in the history
We had some custom handling for existing MemoryEffects but we now move
it to the place we check other existing attributes before we manifest
new ones. If we later decide to curb duplication (of attributes on the
call site and callee), we can do that at a single location and for all
attributes.

The test changes basically add known `memory` callee information to the
call sites.
  • Loading branch information
jdoerfert committed Jul 3, 2023
1 parent 9987646 commit b672c60
Show file tree
Hide file tree
Showing 58 changed files with 582 additions and 627 deletions.
16 changes: 15 additions & 1 deletion llvm/lib/Transforms/IPO/Attributor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
#include "llvm/Support/DebugCounter.h"
#include "llvm/Support/FileSystem.h"
#include "llvm/Support/GraphWriter.h"
#include "llvm/Support/ModRef.h"
#include "llvm/Support/raw_ostream.h"
#include "llvm/Transforms/Utils/BasicBlockUtils.h"
#include "llvm/Transforms/Utils/Cloning.h"
Expand Down Expand Up @@ -930,10 +931,23 @@ static bool addIfNotExistent(LLVMContext &Ctx, const Attribute &Attr,
}
if (Attr.isIntAttribute()) {
Attribute::AttrKind Kind = Attr.getKindAsEnum();
if (Attrs.hasAttributeAtIndex(AttrIdx, Kind))
if (Attrs.hasAttributeAtIndex(AttrIdx, Kind)) {
if (!ForceReplace && Kind == Attribute::Memory) {
MemoryEffects ExistingME =
Attrs.getAttributeAtIndex(AttrIdx, Attribute::Memory)
.getMemoryEffects();
MemoryEffects ME = Attr.getMemoryEffects() & ExistingME;
if (ME == ExistingME)
return false;
Attrs = Attrs.removeAttributeAtIndex(Ctx, AttrIdx, Kind);
Attrs = Attrs.addAttributesAtIndex(Ctx, AttrIdx,
AttrBuilder(Ctx).addMemoryAttr(ME));
return true;
}
if (!ForceReplace &&
isEqualOrWorse(Attr, Attrs.getAttributeAtIndex(AttrIdx, Kind)))
return false;
}
Attrs = Attrs.removeAttributeAtIndex(Ctx, AttrIdx, Kind);
Attrs = Attrs.addAttributeAtIndex(Ctx, AttrIdx, Attr);
return true;
Expand Down
38 changes: 6 additions & 32 deletions llvm/lib/Transforms/IPO/AttributorAttributes.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8006,6 +8006,7 @@ struct AAMemoryBehaviorArgument : AAMemoryBehaviorFloating {
removeKnownBits(NO_WRITES);
removeAssumedBits(NO_WRITES);
}
getIRPosition().removeAttrs(AttrKinds);
return AAMemoryBehaviorFloating::manifest(A);
}

Expand Down Expand Up @@ -8110,16 +8111,9 @@ struct AAMemoryBehaviorFunction final : public AAMemoryBehaviorImpl {
else if (isAssumedWriteOnly())
ME = MemoryEffects::writeOnly();

// Intersect with existing memory attribute, as we currently deduce the
// location and modref portion separately.
MemoryEffects ExistingME = F.getMemoryEffects();
ME &= ExistingME;
if (ME == ExistingME)
return ChangeStatus::UNCHANGED;

return IRAttributeManifest::manifestAttrs(
A, getIRPosition(), Attribute::getWithMemoryEffects(F.getContext(), ME),
/*ForceReplace*/ true);
A, getIRPosition(),
Attribute::getWithMemoryEffects(F.getContext(), ME));
}

/// See AbstractAttribute::trackStatistics()
Expand Down Expand Up @@ -8165,17 +8159,10 @@ struct AAMemoryBehaviorCallSite final : AAMemoryBehaviorImpl {
else if (isAssumedWriteOnly())
ME = MemoryEffects::writeOnly();

// Intersect with existing memory attribute, as we currently deduce the
// location and modref portion separately.
MemoryEffects ExistingME = CB.getMemoryEffects();
ME &= ExistingME;
if (ME == ExistingME)
return ChangeStatus::UNCHANGED;

getIRPosition().removeAttrs(AttrKinds);
return IRAttributeManifest::manifestAttrs(
A, getIRPosition(),
Attribute::getWithMemoryEffects(CB.getContext(), ME),
/*ForceReplace*/ true);
Attribute::getWithMemoryEffects(CB.getContext(), ME));
}

/// See AbstractAttribute::trackStatistics()
Expand Down Expand Up @@ -8539,22 +8526,9 @@ struct AAMemoryLocationImpl : public AAMemoryLocation {
return ChangeStatus::UNCHANGED;
MemoryEffects ME = DeducedAttrs[0].getMemoryEffects();

// Intersect with existing memory attribute, as we currently deduce the
// location and modref portion separately.
SmallVector<Attribute, 1> ExistingAttrs;
IRP.getAttrs({Attribute::Memory}, ExistingAttrs,
/* IgnoreSubsumingPositions */ true);
if (ExistingAttrs.size() == 1) {
MemoryEffects ExistingME = ExistingAttrs[0].getMemoryEffects();
ME &= ExistingME;
if (ME == ExistingME)
return ChangeStatus::UNCHANGED;
}

return IRAttributeManifest::manifestAttrs(
A, IRP,
Attribute::getWithMemoryEffects(IRP.getAnchorValue().getContext(), ME),
/*ForceReplace*/ true);
Attribute::getWithMemoryEffects(IRP.getAnchorValue().getContext(), ME));
}

/// See AAMemoryLocation::checkForAllAccessesToMemoryKind(...).
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ define i32 @foo(ptr %A) {
; CGSCC: Function Attrs: mustprogress nofree nosync nounwind willreturn memory(argmem: read)
; CGSCC-LABEL: define {{[^@]+}}@foo
; CGSCC-SAME: (ptr nocapture nofree noundef nonnull readonly align 4 dereferenceable(4) [[A:%.*]]) #[[ATTR1:[0-9]+]] {
; CGSCC-NEXT: [[X:%.*]] = call i32 @callee(ptr nocapture nofree noundef nonnull readonly align 4 dereferenceable(4) [[A]])
; CGSCC-NEXT: [[X:%.*]] = call i32 @callee(ptr nocapture nofree noundef nonnull readonly align 4 dereferenceable(4) [[A]]) #[[ATTR2:[0-9]+]]
; CGSCC-NEXT: ret i32 [[X]]
;
%X = call i32 @callee(i1 false, ptr %A) ; <i32> [#uses=1]
Expand All @@ -54,8 +54,9 @@ define i32 @foo(ptr %A) {

;.
; TUNIT: attributes #[[ATTR0]] = { mustprogress nofree norecurse nosync nounwind willreturn memory(argmem: read) }
; TUNIT: attributes #[[ATTR1]] = { nofree nosync nounwind willreturn }
; TUNIT: attributes #[[ATTR1]] = { nofree nosync nounwind willreturn memory(read) }
;.
; CGSCC: attributes #[[ATTR0]] = { mustprogress nofree norecurse nosync nounwind willreturn memory(argmem: read) }
; CGSCC: attributes #[[ATTR1]] = { mustprogress nofree nosync nounwind willreturn memory(argmem: read) }
; CGSCC: attributes #[[ATTR2]] = { memory(read) }
;.
22 changes: 12 additions & 10 deletions llvm/test/Transforms/Attributor/ArgumentPromotion/X86/attributes.ll
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@ define void @no_promote(ptr %arg) #1 {
; TUNIT-NEXT: bb:
; TUNIT-NEXT: [[TMP:%.*]] = alloca <4 x i64>, align 32
; TUNIT-NEXT: [[TMP2:%.*]] = alloca <4 x i64>, align 32
; TUNIT-NEXT: call void @llvm.memset.p0.i64(ptr noalias nocapture nofree noundef nonnull writeonly align 32 dereferenceable(32) [[TMP]], i8 noundef 0, i64 noundef 32, i1 noundef false)
; TUNIT-NEXT: call fastcc void @no_promote_avx2(ptr noalias nocapture nofree noundef nonnull writeonly align 32 dereferenceable(32) [[TMP2]], ptr noalias nocapture nofree noundef nonnull readonly align 32 dereferenceable(32) [[TMP]]) #[[ATTR3:[0-9]+]]
; TUNIT-NEXT: call void @llvm.memset.p0.i64(ptr noalias nocapture nofree noundef nonnull writeonly align 32 dereferenceable(32) [[TMP]], i8 noundef 0, i64 noundef 32, i1 noundef false) #[[ATTR3:[0-9]+]]
; TUNIT-NEXT: call fastcc void @no_promote_avx2(ptr noalias nocapture nofree noundef nonnull writeonly align 32 dereferenceable(32) [[TMP2]], ptr noalias nocapture nofree noundef nonnull readonly align 32 dereferenceable(32) [[TMP]]) #[[ATTR4:[0-9]+]]
; TUNIT-NEXT: [[TMP4:%.*]] = load <4 x i64>, ptr [[TMP2]], align 32
; TUNIT-NEXT: store <4 x i64> [[TMP4]], ptr [[ARG]], align 2
; TUNIT-NEXT: ret void
Expand All @@ -40,8 +40,8 @@ define void @no_promote(ptr %arg) #1 {
; CGSCC-NEXT: bb:
; CGSCC-NEXT: [[TMP:%.*]] = alloca <4 x i64>, align 32
; CGSCC-NEXT: [[TMP2:%.*]] = alloca <4 x i64>, align 32
; CGSCC-NEXT: call void @llvm.memset.p0.i64(ptr noalias nocapture nofree noundef nonnull writeonly align 32 dereferenceable(32) [[TMP]], i8 noundef 0, i64 noundef 32, i1 noundef false)
; CGSCC-NEXT: call fastcc void @no_promote_avx2(ptr noalias nocapture nofree noundef nonnull writeonly align 32 dereferenceable(32) [[TMP2]], ptr noalias nocapture nofree noundef nonnull readonly align 32 dereferenceable(32) [[TMP]]) #[[ATTR3:[0-9]+]]
; CGSCC-NEXT: call void @llvm.memset.p0.i64(ptr noalias nocapture nofree noundef nonnull writeonly align 32 dereferenceable(32) [[TMP]], i8 noundef 0, i64 noundef 32, i1 noundef false) #[[ATTR3:[0-9]+]]
; CGSCC-NEXT: call fastcc void @no_promote_avx2(ptr noalias nocapture nofree noundef nonnull writeonly align 32 dereferenceable(32) [[TMP2]], ptr noalias nocapture nofree noundef nonnull readonly align 32 dereferenceable(32) [[TMP]]) #[[ATTR4:[0-9]+]]
; CGSCC-NEXT: [[TMP4:%.*]] = load <4 x i64>, ptr [[TMP2]], align 32
; CGSCC-NEXT: store <4 x i64> [[TMP4]], ptr [[ARG]], align 2
; CGSCC-NEXT: ret void
Expand Down Expand Up @@ -80,9 +80,9 @@ define void @promote(ptr %arg) #0 {
; TUNIT-NEXT: bb:
; TUNIT-NEXT: [[TMP:%.*]] = alloca <4 x i64>, align 32
; TUNIT-NEXT: [[TMP2:%.*]] = alloca <4 x i64>, align 32
; TUNIT-NEXT: call void @llvm.memset.p0.i64(ptr noalias nocapture nofree noundef nonnull writeonly align 32 dereferenceable(32) [[TMP]], i8 noundef 0, i64 noundef 32, i1 noundef false)
; TUNIT-NEXT: call void @llvm.memset.p0.i64(ptr noalias nocapture nofree noundef nonnull writeonly align 32 dereferenceable(32) [[TMP]], i8 noundef 0, i64 noundef 32, i1 noundef false) #[[ATTR3]]
; TUNIT-NEXT: [[TMP0:%.*]] = load <4 x i64>, ptr [[TMP]], align 32
; TUNIT-NEXT: call fastcc void @promote_avx2(ptr noalias nocapture nofree noundef nonnull writeonly align 32 dereferenceable(32) [[TMP2]], <4 x i64> [[TMP0]]) #[[ATTR3]]
; TUNIT-NEXT: call fastcc void @promote_avx2(ptr noalias nocapture nofree noundef nonnull writeonly align 32 dereferenceable(32) [[TMP2]], <4 x i64> [[TMP0]]) #[[ATTR4]]
; TUNIT-NEXT: [[TMP4:%.*]] = load <4 x i64>, ptr [[TMP2]], align 32
; TUNIT-NEXT: store <4 x i64> [[TMP4]], ptr [[ARG]], align 2
; TUNIT-NEXT: ret void
Expand All @@ -93,9 +93,9 @@ define void @promote(ptr %arg) #0 {
; CGSCC-NEXT: bb:
; CGSCC-NEXT: [[TMP:%.*]] = alloca <4 x i64>, align 32
; CGSCC-NEXT: [[TMP2:%.*]] = alloca <4 x i64>, align 32
; CGSCC-NEXT: call void @llvm.memset.p0.i64(ptr noalias nocapture nofree noundef nonnull writeonly align 32 dereferenceable(32) [[TMP]], i8 noundef 0, i64 noundef 32, i1 noundef false)
; CGSCC-NEXT: call void @llvm.memset.p0.i64(ptr noalias nocapture nofree noundef nonnull writeonly align 32 dereferenceable(32) [[TMP]], i8 noundef 0, i64 noundef 32, i1 noundef false) #[[ATTR3]]
; CGSCC-NEXT: [[TMP0:%.*]] = load <4 x i64>, ptr [[TMP]], align 32
; CGSCC-NEXT: call fastcc void @promote_avx2(ptr noalias nocapture nofree noundef nonnull writeonly align 32 dereferenceable(32) [[TMP2]], <4 x i64> [[TMP0]]) #[[ATTR3]]
; CGSCC-NEXT: call fastcc void @promote_avx2(ptr noalias nocapture nofree noundef nonnull writeonly align 32 dereferenceable(32) [[TMP2]], <4 x i64> [[TMP0]]) #[[ATTR4]]
; CGSCC-NEXT: [[TMP4:%.*]] = load <4 x i64>, ptr [[TMP2]], align 32
; CGSCC-NEXT: store <4 x i64> [[TMP4]], ptr [[ARG]], align 2
; CGSCC-NEXT: ret void
Expand All @@ -120,10 +120,12 @@ attributes #2 = { argmemonly nounwind }
; TUNIT: attributes #[[ATTR0]] = { inlinehint mustprogress nofree norecurse nosync nounwind willreturn memory(argmem: readwrite) uwtable "target-features"="+avx2" }
; TUNIT: attributes #[[ATTR1]] = { mustprogress nofree norecurse nosync nounwind willreturn memory(argmem: readwrite) uwtable }
; TUNIT: attributes #[[ATTR2:[0-9]+]] = { nocallback nofree nounwind willreturn memory(argmem: write) }
; TUNIT: attributes #[[ATTR3]] = { nofree nosync nounwind willreturn }
; TUNIT: attributes #[[ATTR3]] = { memory(write) }
; TUNIT: attributes #[[ATTR4]] = { nofree nosync nounwind willreturn }
;.
; CGSCC: attributes #[[ATTR0]] = { inlinehint mustprogress nofree norecurse nosync nounwind willreturn memory(argmem: readwrite) uwtable "target-features"="+avx2" }
; CGSCC: attributes #[[ATTR1]] = { mustprogress nofree nosync nounwind willreturn memory(argmem: readwrite) uwtable }
; CGSCC: attributes #[[ATTR2:[0-9]+]] = { nocallback nofree nounwind willreturn memory(argmem: write) }
; CGSCC: attributes #[[ATTR3]] = { nounwind }
; CGSCC: attributes #[[ATTR3]] = { memory(write) }
; CGSCC: attributes #[[ATTR4]] = { nounwind }
;.
Loading

0 comments on commit b672c60

Please sign in to comment.