Skip to content

[Inliner] Fix bug where attributes are propagated incorrectly #109347

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

Merged
Merged
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
20 changes: 16 additions & 4 deletions llvm/lib/Transforms/Utils/InlineFunction.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1352,7 +1352,8 @@ static bool MayContainThrowingOrExitingCallAfterCB(CallBase *Begin,
// 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) {
ValueToValueMapTy &VMap,
ClonedCodeInfo &InlinedFunctionInfo) {
auto *CalledFunction = CB.getCalledFunction();
auto &Context = CalledFunction->getContext();

Expand Down Expand Up @@ -1383,6 +1384,11 @@ static void AddParamAndFnBasicAttributes(const CallBase &CB,
auto *NewInnerCB = dyn_cast_or_null<CallBase>(VMap.lookup(InnerCB));
if (!NewInnerCB)
continue;
// The InnerCB might have be simplified during the inlining
// process which can make propagation incorrect.
if (InlinedFunctionInfo.isSimplified(InnerCB, 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.
Expand Down Expand Up @@ -1458,7 +1464,8 @@ static AttrBuilder IdentifyValidPoisonGeneratingAttributes(CallBase &CB) {
return Valid;
}

static void AddReturnAttributes(CallBase &CB, ValueToValueMapTy &VMap) {
static void AddReturnAttributes(CallBase &CB, ValueToValueMapTy &VMap,
ClonedCodeInfo &InlinedFunctionInfo) {
AttrBuilder ValidUB = IdentifyValidUBGeneratingAttributes(CB);
AttrBuilder ValidPG = IdentifyValidPoisonGeneratingAttributes(CB);
if (!ValidUB.hasAttributes() && !ValidPG.hasAttributes())
Expand All @@ -1477,6 +1484,11 @@ static void AddReturnAttributes(CallBase &CB, ValueToValueMapTy &VMap) {
auto *NewRetVal = dyn_cast_or_null<CallBase>(VMap.lookup(RetVal));
if (!NewRetVal)
continue;

// The RetVal might have be simplified during the inlining
// process which can make propagation incorrect.
if (InlinedFunctionInfo.isSimplified(RetVal, NewRetVal))
continue;
// Backward propagation of attributes to the returned value may be incorrect
// if it is control flow dependent.
// Consider:
Expand Down Expand Up @@ -2693,11 +2705,11 @@ llvm::InlineResult llvm::InlineFunction(CallBase &CB, InlineFunctionInfo &IFI,

// Clone return attributes on the callsite into the calls within the inlined
// function which feed into its return value.
AddReturnAttributes(CB, VMap);
AddReturnAttributes(CB, VMap, InlinedFunctionInfo);

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

propagateMemProfMetadata(CalledFunc, CB,
InlinedFunctionInfo.ContainsMemProfMetadata, VMap);
Expand Down
21 changes: 21 additions & 0 deletions llvm/test/Transforms/Inline/access-attributes-prop.ll
Original file line number Diff line number Diff line change
Expand Up @@ -559,3 +559,24 @@ define void @prop_byval_readonly(ptr %p) {
call void @foo_byval_readonly(ptr %p)
ret void
}

define ptr @caller_bad_param_prop(ptr %p1, ptr %p2, i64 %x) {
; CHECK-LABEL: define {{[^@]+}}@caller_bad_param_prop
; CHECK-SAME: (ptr [[P1:%.*]], ptr [[P2:%.*]], i64 [[X:%.*]]) {
; CHECK-NEXT: [[TMP1:%.*]] = call ptr [[P1]](i64 [[X]], ptr [[P2]])
; CHECK-NEXT: ret ptr [[TMP1]]
;
%1 = call ptr %p1(i64 %x, ptr %p2)
%2 = call ptr @callee_bad_param_prop(ptr %1)
ret ptr %2
}

define ptr @callee_bad_param_prop(ptr readonly %x) {
; CHECK-LABEL: define {{[^@]+}}@callee_bad_param_prop
; CHECK-SAME: (ptr readonly [[X:%.*]]) {
; CHECK-NEXT: [[R:%.*]] = tail call ptr @llvm.ptrmask.p0.i64(ptr [[X]], i64 -1)
; CHECK-NEXT: ret ptr [[R]]
;
%r = tail call ptr @llvm.ptrmask(ptr %x, i64 -1)
ret ptr %r
}
40 changes: 40 additions & 0 deletions llvm/test/Transforms/Inline/ret_attr_align_and_noundef.ll
Original file line number Diff line number Diff line change
Expand Up @@ -421,3 +421,43 @@ define i8 @caller16_not_intersecting_ranges() {
call void @use.val(i8 %r)
ret i8 %r
}


define ptr @caller_bad_ret_prop(ptr %p1, ptr %p2, i64 %x, ptr %other) {
; CHECK-LABEL: define ptr @caller_bad_ret_prop
; CHECK-SAME: (ptr [[P1:%.*]], ptr [[P2:%.*]], i64 [[X:%.*]], ptr [[OTHER:%.*]]) {
; CHECK-NEXT: [[TMP1:%.*]] = call noundef ptr [[P1]](i64 [[X]], ptr [[P2]])
; CHECK-NEXT: [[CMP_I:%.*]] = icmp eq ptr [[TMP1]], null
; CHECK-NEXT: br i1 [[CMP_I]], label [[T_I:%.*]], label [[F_I:%.*]]
; CHECK: T.i:
; CHECK-NEXT: br label [[CALLEE_BAD_RET_PROP_EXIT:%.*]]
; CHECK: F.i:
; CHECK-NEXT: br label [[CALLEE_BAD_RET_PROP_EXIT]]
; CHECK: callee_bad_ret_prop.exit:
; CHECK-NEXT: [[TMP2:%.*]] = phi ptr [ [[OTHER]], [[T_I]] ], [ [[TMP1]], [[F_I]] ]
; CHECK-NEXT: ret ptr [[TMP2]]
;
%1 = call noundef ptr %p1(i64 %x, ptr %p2)
%2 = call nonnull ptr @callee_bad_ret_prop(ptr %1, ptr %other)
ret ptr %2
}

define ptr @callee_bad_ret_prop(ptr %x, ptr %other) {
; CHECK-LABEL: define ptr @callee_bad_ret_prop
; CHECK-SAME: (ptr [[X:%.*]], ptr [[OTHER:%.*]]) {
; CHECK-NEXT: [[CMP:%.*]] = icmp eq ptr [[X]], null
; CHECK-NEXT: br i1 [[CMP]], label [[T:%.*]], label [[F:%.*]]
; CHECK: T:
; CHECK-NEXT: ret ptr [[OTHER]]
; CHECK: F:
; CHECK-NEXT: [[R:%.*]] = tail call ptr @llvm.ptrmask.p0.i64(ptr [[X]], i64 -1)
; CHECK-NEXT: ret ptr [[R]]
;
%cmp = icmp eq ptr %x, null
br i1 %cmp, label %T, label %F
T:
ret ptr %other
F:
%r = tail call ptr @llvm.ptrmask(ptr %x, i64 -1)
ret ptr %r
}
Loading