Skip to content

Commit ae11d1f

Browse files
committed
[Inliner] Don't propagate access attr to byval params
We previously only handled the case where the `byval` attr was in the callbase's param attr list. This PR also handles the case if the `ByVal` was a param attr on the function's param attr list.
1 parent 73e4a18 commit ae11d1f

File tree

2 files changed

+12
-7
lines changed

2 files changed

+12
-7
lines changed

llvm/lib/Transforms/Utils/InlineFunction.cpp

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1398,7 +1398,7 @@ static void AddParamAndFnBasicAttributes(const CallBase &CB,
13981398
if (!Arg)
13991399
continue;
14001400

1401-
if (AL.hasParamAttr(I, Attribute::ByVal))
1401+
if (NewInnerCB->getParamAttr(I, Attribute::ByVal).isValid())
14021402
// It's unsound to propagate memory attributes to byval arguments.
14031403
// Even if CalledFunction doesn't e.g. write to the argument,
14041404
// the call to NewInnerCB may write to its by-value copy.
@@ -1407,24 +1407,29 @@ static void AddParamAndFnBasicAttributes(const CallBase &CB,
14071407
unsigned ArgNo = Arg->getArgNo();
14081408
// If so, propagate its access attributes.
14091409
AL = AL.addParamAttributes(Context, I, ValidParamAttrs[ArgNo]);
1410+
auto NewCBHasParamAttr = [&](Attribute::AttrKind Kind) {
1411+
return AL.hasParamAttr(I, Kind) ||
1412+
NewInnerCB->getParamAttr(I, Kind).isValid();
1413+
};
1414+
14101415
// We can have conflicting attributes from the inner callsite and
14111416
// to-be-inlined callsite. In that case, choose the most
14121417
// restrictive.
14131418

14141419
// readonly + writeonly means we can never deref so make readnone.
1415-
if (AL.hasParamAttr(I, Attribute::ReadOnly) &&
1416-
AL.hasParamAttr(I, Attribute::WriteOnly))
1420+
if (NewCBHasParamAttr(Attribute::ReadOnly) &&
1421+
NewCBHasParamAttr(Attribute::WriteOnly))
14171422
AL = AL.addParamAttribute(Context, I, Attribute::ReadNone);
14181423

14191424
// If have readnone, need to clear readonly/writeonly
1420-
if (AL.hasParamAttr(I, Attribute::ReadNone)) {
1425+
if (NewCBHasParamAttr(Attribute::ReadNone)) {
14211426
AL = AL.removeParamAttribute(Context, I, Attribute::ReadOnly);
14221427
AL = AL.removeParamAttribute(Context, I, Attribute::WriteOnly);
14231428
}
14241429

14251430
// Writable cannot exist in conjunction w/ readonly/readnone
1426-
if (AL.hasParamAttr(I, Attribute::ReadOnly) ||
1427-
AL.hasParamAttr(I, Attribute::ReadNone))
1431+
if (NewCBHasParamAttr(Attribute::ReadOnly) ||
1432+
NewCBHasParamAttr(Attribute::ReadNone))
14281433
AL = AL.removeParamAttribute(Context, I, Attribute::Writable);
14291434
}
14301435
NewInnerCB->setAttributes(AL);

llvm/test/Transforms/Inline/access-attributes-prop.ll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -594,7 +594,7 @@ define dso_local void @foo_byval_readonly2(ptr readonly %p) {
594594
define void @prop_byval_readonly2(ptr %p) {
595595
; CHECK-LABEL: define {{[^@]+}}@prop_byval_readonly2
596596
; CHECK-SAME: (ptr [[P:%.*]]) {
597-
; CHECK-NEXT: call void @bar4(ptr readonly [[P]])
597+
; CHECK-NEXT: call void @bar4(ptr [[P]])
598598
; CHECK-NEXT: ret void
599599
;
600600
call void @foo_byval_readonly2(ptr %p)

0 commit comments

Comments
 (0)