Skip to content

Recommit "[Inliner] Propagate callee argument memory access attributes before inlining" (2nd Try) #95888

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.

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

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
81 changes: 81 additions & 0 deletions llvm/lib/Transforms/Utils/InlineFunction.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1344,6 +1344,83 @@ static bool MayContainThrowingOrExitingCallAfterCB(CallBase *Begin,
++BeginIt, End->getIterator(), InlinerAttributeWindow + 1);
}

// Add attributes from CB params and Fn attributes that can always be propagated
// to the corresponding argument / inner callbases.
static void AddParamAndFnBasicAttributes(const CallBase &CB,
ValueToValueMapTy &VMap) {
auto *CalledFunction = CB.getCalledFunction();
auto &Context = CalledFunction->getContext();

// Collect valid attributes for all params.
SmallVector<AttrBuilder> ValidParamAttrs;
bool HasAttrToPropagate = false;

for (unsigned I = 0, E = CB.arg_size(); I < E; ++I) {
ValidParamAttrs.emplace_back(AttrBuilder{CB.getContext()});
// Access attributes can be propagated to any param with the same underlying
// object as the argument.
if (CB.paramHasAttr(I, Attribute::ReadNone))
ValidParamAttrs.back().addAttribute(Attribute::ReadNone);
if (CB.paramHasAttr(I, Attribute::ReadOnly))
ValidParamAttrs.back().addAttribute(Attribute::ReadOnly);
HasAttrToPropagate |= ValidParamAttrs.back().hasAttributes();
}

// Won't be able to propagate anything.
if (!HasAttrToPropagate)
return;

for (BasicBlock &BB : *CalledFunction) {
for (Instruction &Ins : BB) {
const auto *InnerCB = dyn_cast<CallBase>(&Ins);
if (!InnerCB)
continue;
auto *NewInnerCB = dyn_cast_or_null<CallBase>(VMap.lookup(InnerCB));
if (!NewInnerCB)
continue;
AttributeList AL = NewInnerCB->getAttributes();
for (unsigned I = 0, E = InnerCB->arg_size(); I < E; ++I) {
// Check if the underlying value for the parameter is an argument.
const Value *UnderlyingV =
getUnderlyingObject(InnerCB->getArgOperand(I));
const Argument *Arg = dyn_cast<Argument>(UnderlyingV);
if (!Arg)
continue;

if (AL.hasParamAttr(I, Attribute::ByVal))
// It's unsound to propagate memory attributes to byval arguments.
// Even if CalledFunction doesn't e.g. write to the argument,
// the call to NewInnerCB may write to its by-value copy.
continue;

unsigned ArgNo = Arg->getArgNo();
// If so, propagate its access attributes.
AL = AL.addParamAttributes(Context, I, ValidParamAttrs[ArgNo]);
// We can have conflicting attributes from the inner callsite and
// to-be-inlined callsite. In that case, choose the most
// restrictive.

// readonly + writeonly means we can never deref so make readnone.
if (AL.hasParamAttr(I, Attribute::ReadOnly) &&
AL.hasParamAttr(I, Attribute::WriteOnly))
AL = AL.addParamAttribute(Context, I, Attribute::ReadNone);

// If have readnone, need to clear readonly/writeonly
if (AL.hasParamAttr(I, Attribute::ReadNone)) {
AL = AL.removeParamAttribute(Context, I, Attribute::ReadOnly);
AL = AL.removeParamAttribute(Context, I, Attribute::WriteOnly);
}

// Writable cannot exist in conjunction w/ readonly/readnone
if (AL.hasParamAttr(I, Attribute::ReadOnly) ||
AL.hasParamAttr(I, Attribute::ReadNone))
AL = AL.removeParamAttribute(Context, I, Attribute::Writable);
}
NewInnerCB->setAttributes(AL);
}
}
}

// Only allow these white listed attributes to be propagated back to the
// callee. This is because other attributes may only be valid on the call
// itself, i.e. attributes such as signext and zeroext.
Expand Down Expand Up @@ -2363,6 +2440,10 @@ llvm::InlineResult llvm::InlineFunction(CallBase &CB, InlineFunctionInfo &IFI,
// function which feed into its return value.
AddReturnAttributes(CB, VMap);

// Clone attributes on the params of the callsite to calls within the
// inlined function which use the same param.
AddParamAndFnBasicAttributes(CB, VMap);

propagateMemProfMetadata(CalledFunc, CB,
InlinedFunctionInfo.ContainsMemProfMetadata, VMap);

Expand Down
42 changes: 31 additions & 11 deletions llvm/test/Transforms/Inline/access-attributes-prop.ll
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
declare void @bar1(ptr %p)
declare void @bar2(ptr %p, ptr %p2)
declare void @bar3(ptr writable %p)
declare void @bar4(ptr byval([4 x i32]) %p)
define dso_local void @foo1_rdonly(ptr readonly %p) {
; CHECK-LABEL: define {{[^@]+}}@foo1_rdonly
; CHECK-SAME: (ptr readonly [[P:%.*]]) {
Expand Down Expand Up @@ -186,10 +187,20 @@ define dso_local void @foo2_through_obj(ptr %p, ptr %p2) {
ret void
}

define dso_local void @foo_byval_readonly(ptr readonly %p) {
; CHECK-LABEL: define {{[^@]+}}@foo_byval_readonly
; CHECK-SAME: (ptr readonly [[P:%.*]]) {
; CHECK-NEXT: call void @bar4(ptr byval([4 x i32]) [[P]])
; CHECK-NEXT: ret void
;
call void @bar4(ptr byval([4 x i32]) %p)
ret void
}

define void @prop_param_func_decl(ptr %p) {
; CHECK-LABEL: define {{[^@]+}}@prop_param_func_decl
; CHECK-SAME: (ptr [[P:%.*]]) {
; CHECK-NEXT: call void @bar1(ptr [[P]])
; CHECK-NEXT: call void @bar1(ptr readonly [[P]])
; CHECK-NEXT: ret void
;
call void @foo1_rdonly(ptr %p)
Expand All @@ -199,7 +210,7 @@ define void @prop_param_func_decl(ptr %p) {
define void @prop_param_callbase_def(ptr %p) {
; CHECK-LABEL: define {{[^@]+}}@prop_param_callbase_def
; CHECK-SAME: (ptr [[P:%.*]]) {
; CHECK-NEXT: call void @bar1(ptr [[P]])
; CHECK-NEXT: call void @bar1(ptr readonly [[P]])
; CHECK-NEXT: call void @bar1(ptr [[P]])
; CHECK-NEXT: ret void
;
Expand All @@ -211,7 +222,7 @@ define void @prop_param_callbase_def(ptr %p) {
define void @prop_param_callbase_def_2x(ptr %p, ptr %p2) {
; CHECK-LABEL: define {{[^@]+}}@prop_param_callbase_def_2x
; CHECK-SAME: (ptr [[P:%.*]], ptr [[P2:%.*]]) {
; CHECK-NEXT: call void @bar2(ptr [[P]], ptr [[P]])
; CHECK-NEXT: call void @bar2(ptr readonly [[P]], ptr readonly [[P]])
; CHECK-NEXT: ret void
;
call void @foo2(ptr readonly %p, ptr %p)
Expand All @@ -223,7 +234,7 @@ define void @prop_param_callbase_def_2x_2(ptr %p, ptr %p2) {
; CHECK-SAME: (ptr [[P:%.*]], ptr [[P2:%.*]]) {
; CHECK-NEXT: [[PP_I:%.*]] = getelementptr i8, ptr [[P]], i64 9
; CHECK-NEXT: [[P2P_I:%.*]] = getelementptr i8, ptr [[P2]], i64 123
; CHECK-NEXT: call void @bar2(ptr [[P2P_I]], ptr [[PP_I]])
; CHECK-NEXT: call void @bar2(ptr [[P2P_I]], ptr readonly [[PP_I]])
; CHECK-NEXT: ret void
;
call void @foo2_through_obj(ptr readonly %p, ptr writeonly %p2)
Expand All @@ -235,7 +246,7 @@ define void @prop_param_callbase_def_2x_incompat(ptr %p, ptr %p2) {
; CHECK-SAME: (ptr [[P:%.*]], ptr [[P2:%.*]]) {
; CHECK-NEXT: [[PP_I:%.*]] = getelementptr i8, ptr [[P]], i64 9
; CHECK-NEXT: [[P2P_I:%.*]] = getelementptr i8, ptr [[P]], i64 123
; CHECK-NEXT: call void @bar2(ptr [[P2P_I]], ptr [[PP_I]])
; CHECK-NEXT: call void @bar2(ptr readonly [[P2P_I]], ptr readnone [[PP_I]])
; CHECK-NEXT: ret void
;
call void @foo2_through_obj(ptr readnone %p, ptr readonly %p)
Expand All @@ -245,7 +256,7 @@ define void @prop_param_callbase_def_2x_incompat(ptr %p, ptr %p2) {
define void @prop_param_callbase_def_2x_incompat_2(ptr %p, ptr %p2) {
; CHECK-LABEL: define {{[^@]+}}@prop_param_callbase_def_2x_incompat_2
; CHECK-SAME: (ptr [[P:%.*]], ptr [[P2:%.*]]) {
; CHECK-NEXT: call void @bar2(ptr [[P]], ptr [[P]])
; CHECK-NEXT: call void @bar2(ptr readonly [[P]], ptr readonly [[P]])
; CHECK-NEXT: ret void
;
call void @foo2(ptr readonly %p, ptr readnone %p)
Expand All @@ -255,7 +266,7 @@ define void @prop_param_callbase_def_2x_incompat_2(ptr %p, ptr %p2) {
define void @prop_param_callbase_def_2x_incompat_3(ptr %p, ptr %p2) {
; CHECK-LABEL: define {{[^@]+}}@prop_param_callbase_def_2x_incompat_3
; CHECK-SAME: (ptr [[P:%.*]], ptr [[P2:%.*]]) {
; CHECK-NEXT: call void @bar2(ptr [[P]], ptr [[P]])
; CHECK-NEXT: call void @bar2(ptr readnone [[P]], ptr readnone [[P]])
; CHECK-NEXT: ret void
;
call void @foo2_2(ptr readonly %p, ptr readnone %p)
Expand All @@ -265,7 +276,7 @@ define void @prop_param_callbase_def_2x_incompat_3(ptr %p, ptr %p2) {
define void @prop_param_callbase_def_1x_partial(ptr %p, ptr %p2) {
; CHECK-LABEL: define {{[^@]+}}@prop_param_callbase_def_1x_partial
; CHECK-SAME: (ptr [[P:%.*]], ptr [[P2:%.*]]) {
; CHECK-NEXT: call void @bar2(ptr [[P]], ptr [[P]])
; CHECK-NEXT: call void @bar2(ptr readonly [[P]], ptr readonly [[P]])
; CHECK-NEXT: ret void
;
call void @foo2(ptr readonly %p, ptr %p)
Expand All @@ -285,7 +296,7 @@ define void @prop_param_callbase_def_1x_partial_2(ptr %p, ptr %p2) {
define void @prop_param_callbase_def_1x_partial_3(ptr %p, ptr %p2) {
; CHECK-LABEL: define {{[^@]+}}@prop_param_callbase_def_1x_partial_3
; CHECK-SAME: (ptr [[P:%.*]], ptr [[P2:%.*]]) {
; CHECK-NEXT: call void @bar2(ptr [[P]], ptr [[P]])
; CHECK-NEXT: call void @bar2(ptr readonly [[P]], ptr readnone [[P]])
; CHECK-NEXT: ret void
;
call void @foo2_3(ptr readonly %p, ptr %p)
Expand Down Expand Up @@ -521,7 +532,7 @@ define void @prop_cb_def_mustprogress(ptr %p) {
define void @prop_no_conflict_writable(ptr %p) {
; CHECK-LABEL: define {{[^@]+}}@prop_no_conflict_writable
; CHECK-SAME: (ptr [[P:%.*]]) {
; CHECK-NEXT: call void @bar1(ptr writable [[P]])
; CHECK-NEXT: call void @bar1(ptr readonly [[P]])
; CHECK-NEXT: ret void
;
call void @foo1_writable(ptr readonly %p)
Expand All @@ -532,10 +543,19 @@ define void @prop_no_conflict_writable(ptr %p) {
define void @prop_no_conflict_writable2(ptr %p) {
; CHECK-LABEL: define {{[^@]+}}@prop_no_conflict_writable2
; CHECK-SAME: (ptr [[P:%.*]]) {
; CHECK-NEXT: call void @bar3(ptr [[P]])
; CHECK-NEXT: call void @bar3(ptr readnone [[P]])
; CHECK-NEXT: ret void
;
call void @foo3_writable(ptr readnone %p)
ret void
}

define void @prop_byval_readonly(ptr %p) {
; CHECK-LABEL: define {{[^@]+}}@prop_byval_readonly
; CHECK-SAME: (ptr [[P:%.*]]) {
; CHECK-NEXT: call void @bar4(ptr byval([4 x i32]) [[P]])
; CHECK-NEXT: ret void
;
call void @foo_byval_readonly(ptr %p)
ret void
}
20 changes: 10 additions & 10 deletions llvm/test/Transforms/Inline/noalias-calls-always.ll
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,11 @@ define void @foo(ptr nocapture %a, ptr nocapture readonly %c, ptr nocapture %b)
; CHECK-NEXT: call void @llvm.experimental.noalias.scope.decl(metadata [[META0:![0-9]+]])
; CHECK-NEXT: call void @llvm.experimental.noalias.scope.decl(metadata [[META3:![0-9]+]])
; CHECK-NEXT: call void @llvm.lifetime.start.p0(i64 512, ptr [[L_I]])
; CHECK-NEXT: call void @llvm.memcpy.p0.p0.i64(ptr align 16 [[A:%.*]], ptr align 16 [[B:%.*]], i64 16, i1 false), !noalias !3
; CHECK-NEXT: call void @llvm.memcpy.p0.p0.i64(ptr align 16 [[B]], ptr align 16 [[C:%.*]], i64 16, i1 false), !noalias !0
; CHECK-NEXT: call void @llvm.memcpy.p0.p0.i64(ptr align 16 [[A]], ptr align 16 [[C]], i64 16, i1 false), !alias.scope !5
; CHECK-NEXT: call void @hey(), !noalias !5
; CHECK-NEXT: call void @llvm.memcpy.p0.p0.i64(ptr align 16 [[L_I]], ptr align 16 [[C]], i64 16, i1 false), !noalias !0
; CHECK-NEXT: call void @llvm.memcpy.p0.p0.i64(ptr align 16 [[A:%.*]], ptr align 16 [[B:%.*]], i64 16, i1 false), !noalias [[META3]]
; CHECK-NEXT: call void @llvm.memcpy.p0.p0.i64(ptr align 16 [[B]], ptr readonly align 16 [[C:%.*]], i64 16, i1 false), !noalias [[META0]]
; CHECK-NEXT: call void @llvm.memcpy.p0.p0.i64(ptr align 16 [[A]], ptr readonly align 16 [[C]], i64 16, i1 false), !alias.scope [[META5:![0-9]+]]
; CHECK-NEXT: call void @hey(), !noalias [[META5]]
; CHECK-NEXT: call void @llvm.memcpy.p0.p0.i64(ptr align 16 [[L_I]], ptr readonly align 16 [[C]], i64 16, i1 false), !noalias [[META0]]
; CHECK-NEXT: call void @llvm.lifetime.end.p0(i64 512, ptr [[L_I]])
; CHECK-NEXT: ret void
;
Expand Down Expand Up @@ -75,11 +75,11 @@ define void @foo_cs(ptr nocapture %a, ptr nocapture readonly %c, ptr nocapture %
; CHECK-NEXT: call void @llvm.experimental.noalias.scope.decl(metadata [[META6:![0-9]+]])
; CHECK-NEXT: call void @llvm.experimental.noalias.scope.decl(metadata [[META9:![0-9]+]])
; CHECK-NEXT: call void @llvm.lifetime.start.p0(i64 512, ptr [[L_I]])
; CHECK-NEXT: call void @llvm.memcpy.p0.p0.i64(ptr align 16 [[A:%.*]], ptr align 16 [[B:%.*]], i64 16, i1 false), !noalias !9
; CHECK-NEXT: call void @llvm.memcpy.p0.p0.i64(ptr align 16 [[B]], ptr align 16 [[C:%.*]], i64 16, i1 false), !noalias !6
; CHECK-NEXT: call void @llvm.memcpy.p0.p0.i64(ptr align 16 [[A]], ptr align 16 [[C]], i64 16, i1 false), !alias.scope !11
; CHECK-NEXT: call void @hey(), !noalias !11
; CHECK-NEXT: call void @llvm.memcpy.p0.p0.i64(ptr align 16 [[L_I]], ptr align 16 [[C]], i64 16, i1 false), !noalias !6
; CHECK-NEXT: call void @llvm.memcpy.p0.p0.i64(ptr align 16 [[A:%.*]], ptr align 16 [[B:%.*]], i64 16, i1 false), !noalias [[META9]]
; CHECK-NEXT: call void @llvm.memcpy.p0.p0.i64(ptr align 16 [[B]], ptr readonly align 16 [[C:%.*]], i64 16, i1 false), !noalias [[META6]]
; CHECK-NEXT: call void @llvm.memcpy.p0.p0.i64(ptr align 16 [[A]], ptr readonly align 16 [[C]], i64 16, i1 false), !alias.scope [[META11:![0-9]+]]
; CHECK-NEXT: call void @hey(), !noalias [[META11]]
; CHECK-NEXT: call void @llvm.memcpy.p0.p0.i64(ptr align 16 [[L_I]], ptr readonly align 16 [[C]], i64 16, i1 false), !noalias [[META6]]
; CHECK-NEXT: call void @llvm.lifetime.end.p0(i64 512, ptr [[L_I]])
; CHECK-NEXT: ret void
;
Expand Down
20 changes: 10 additions & 10 deletions llvm/test/Transforms/Inline/noalias-calls.ll
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,11 @@ define void @foo(ptr nocapture %a, ptr nocapture readonly %c, ptr nocapture %b)
; CHECK-NEXT: call void @llvm.experimental.noalias.scope.decl(metadata [[META0:![0-9]+]])
; CHECK-NEXT: call void @llvm.experimental.noalias.scope.decl(metadata [[META3:![0-9]+]])
; CHECK-NEXT: call void @llvm.lifetime.start.p0(i64 512, ptr [[L_I]])
; CHECK-NEXT: call void @llvm.memcpy.p0.p0.i64(ptr align 16 [[A]], ptr align 16 [[B]], i64 16, i1 false), !noalias !3
; CHECK-NEXT: call void @llvm.memcpy.p0.p0.i64(ptr align 16 [[B]], ptr align 16 [[C]], i64 16, i1 false), !noalias !0
; CHECK-NEXT: call void @llvm.memcpy.p0.p0.i64(ptr align 16 [[A]], ptr align 16 [[C]], i64 16, i1 false), !alias.scope !5
; CHECK-NEXT: call void @hey(), !noalias !5
; CHECK-NEXT: call void @llvm.memcpy.p0.p0.i64(ptr align 16 [[L_I]], ptr align 16 [[C]], i64 16, i1 false), !noalias !0
; CHECK-NEXT: call void @llvm.memcpy.p0.p0.i64(ptr align 16 [[A]], ptr align 16 [[B]], i64 16, i1 false), !noalias [[META3]]
; CHECK-NEXT: call void @llvm.memcpy.p0.p0.i64(ptr align 16 [[B]], ptr readonly align 16 [[C]], i64 16, i1 false), !noalias [[META0]]
; CHECK-NEXT: call void @llvm.memcpy.p0.p0.i64(ptr align 16 [[A]], ptr readonly align 16 [[C]], i64 16, i1 false), !alias.scope [[META5:![0-9]+]]
; CHECK-NEXT: call void @hey(), !noalias [[META5]]
; CHECK-NEXT: call void @llvm.memcpy.p0.p0.i64(ptr align 16 [[L_I]], ptr readonly align 16 [[C]], i64 16, i1 false), !noalias [[META0]]
; CHECK-NEXT: call void @llvm.lifetime.end.p0(i64 512, ptr [[L_I]])
; CHECK-NEXT: ret void
;
Expand Down Expand Up @@ -80,11 +80,11 @@ define void @foo_cs(ptr nocapture %a, ptr nocapture readonly %c, ptr nocapture %
; CHECK-NEXT: call void @llvm.experimental.noalias.scope.decl(metadata [[META6:![0-9]+]])
; CHECK-NEXT: call void @llvm.experimental.noalias.scope.decl(metadata [[META9:![0-9]+]])
; CHECK-NEXT: call void @llvm.lifetime.start.p0(i64 512, ptr [[L_I]])
; CHECK-NEXT: call void @llvm.memcpy.p0.p0.i64(ptr align 16 [[A]], ptr align 16 [[B]], i64 16, i1 false), !noalias !9
; CHECK-NEXT: call void @llvm.memcpy.p0.p0.i64(ptr align 16 [[B]], ptr align 16 [[C]], i64 16, i1 false), !noalias !6
; CHECK-NEXT: call void @llvm.memcpy.p0.p0.i64(ptr align 16 [[A]], ptr align 16 [[C]], i64 16, i1 false), !alias.scope !11
; CHECK-NEXT: call void @hey(), !noalias !11
; CHECK-NEXT: call void @llvm.memcpy.p0.p0.i64(ptr align 16 [[L_I]], ptr align 16 [[C]], i64 16, i1 false), !noalias !6
; CHECK-NEXT: call void @llvm.memcpy.p0.p0.i64(ptr align 16 [[A]], ptr align 16 [[B]], i64 16, i1 false), !noalias [[META9]]
; CHECK-NEXT: call void @llvm.memcpy.p0.p0.i64(ptr align 16 [[B]], ptr readonly align 16 [[C]], i64 16, i1 false), !noalias [[META6]]
; CHECK-NEXT: call void @llvm.memcpy.p0.p0.i64(ptr align 16 [[A]], ptr readonly align 16 [[C]], i64 16, i1 false), !alias.scope [[META11:![0-9]+]]
; CHECK-NEXT: call void @hey(), !noalias [[META11]]
; CHECK-NEXT: call void @llvm.memcpy.p0.p0.i64(ptr align 16 [[L_I]], ptr readonly align 16 [[C]], i64 16, i1 false), !noalias [[META6]]
; CHECK-NEXT: call void @llvm.lifetime.end.p0(i64 512, ptr [[L_I]])
; CHECK-NEXT: ret void
;
Expand Down
1 change: 1 addition & 0 deletions llvm/test/Transforms/PhaseOrdering/pr95152.ll
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

; Make sure that interaction of "writable" with various passes does not
; result in the elimination of the store prior to @j().
; FIXME: This is a miscompile.

declare void @use(i64)

Expand Down
Loading