Skip to content

Commit a9352a0

Browse files
authored
[Inliner] Fix bug where attributes are propagated incorrectly (#109347)
- **[Inliner] Add tests for incorrect propagation of return attrs; NFC** - **[Inliner] Fix bug where attributes are propagated incorrectly** The bug stems from the fact that we assume the new (inlined) callsite is calling the same function as the original (callee) callsite. While this is typically the case, since `VMap` simplifies the new instructions, callee intrinsics callsites can end up not corresponding with the same function. This can lead to buggy propagation.
1 parent 0a84f12 commit a9352a0

File tree

3 files changed

+77
-4
lines changed

3 files changed

+77
-4
lines changed

llvm/lib/Transforms/Utils/InlineFunction.cpp

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1352,7 +1352,8 @@ static bool MayContainThrowingOrExitingCallAfterCB(CallBase *Begin,
13521352
// Add attributes from CB params and Fn attributes that can always be propagated
13531353
// to the corresponding argument / inner callbases.
13541354
static void AddParamAndFnBasicAttributes(const CallBase &CB,
1355-
ValueToValueMapTy &VMap) {
1355+
ValueToValueMapTy &VMap,
1356+
ClonedCodeInfo &InlinedFunctionInfo) {
13561357
auto *CalledFunction = CB.getCalledFunction();
13571358
auto &Context = CalledFunction->getContext();
13581359

@@ -1383,6 +1384,11 @@ static void AddParamAndFnBasicAttributes(const CallBase &CB,
13831384
auto *NewInnerCB = dyn_cast_or_null<CallBase>(VMap.lookup(InnerCB));
13841385
if (!NewInnerCB)
13851386
continue;
1387+
// The InnerCB might have be simplified during the inlining
1388+
// process which can make propagation incorrect.
1389+
if (InlinedFunctionInfo.isSimplified(InnerCB, NewInnerCB))
1390+
continue;
1391+
13861392
AttributeList AL = NewInnerCB->getAttributes();
13871393
for (unsigned I = 0, E = InnerCB->arg_size(); I < E; ++I) {
13881394
// Check if the underlying value for the parameter is an argument.
@@ -1458,7 +1464,8 @@ static AttrBuilder IdentifyValidPoisonGeneratingAttributes(CallBase &CB) {
14581464
return Valid;
14591465
}
14601466

1461-
static void AddReturnAttributes(CallBase &CB, ValueToValueMapTy &VMap) {
1467+
static void AddReturnAttributes(CallBase &CB, ValueToValueMapTy &VMap,
1468+
ClonedCodeInfo &InlinedFunctionInfo) {
14621469
AttrBuilder ValidUB = IdentifyValidUBGeneratingAttributes(CB);
14631470
AttrBuilder ValidPG = IdentifyValidPoisonGeneratingAttributes(CB);
14641471
if (!ValidUB.hasAttributes() && !ValidPG.hasAttributes())
@@ -1477,6 +1484,11 @@ static void AddReturnAttributes(CallBase &CB, ValueToValueMapTy &VMap) {
14771484
auto *NewRetVal = dyn_cast_or_null<CallBase>(VMap.lookup(RetVal));
14781485
if (!NewRetVal)
14791486
continue;
1487+
1488+
// The RetVal might have be simplified during the inlining
1489+
// process which can make propagation incorrect.
1490+
if (InlinedFunctionInfo.isSimplified(RetVal, NewRetVal))
1491+
continue;
14801492
// Backward propagation of attributes to the returned value may be incorrect
14811493
// if it is control flow dependent.
14821494
// Consider:
@@ -2693,11 +2705,11 @@ llvm::InlineResult llvm::InlineFunction(CallBase &CB, InlineFunctionInfo &IFI,
26932705

26942706
// Clone return attributes on the callsite into the calls within the inlined
26952707
// function which feed into its return value.
2696-
AddReturnAttributes(CB, VMap);
2708+
AddReturnAttributes(CB, VMap, InlinedFunctionInfo);
26972709

26982710
// Clone attributes on the params of the callsite to calls within the
26992711
// inlined function which use the same param.
2700-
AddParamAndFnBasicAttributes(CB, VMap);
2712+
AddParamAndFnBasicAttributes(CB, VMap, InlinedFunctionInfo);
27012713

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

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

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -559,3 +559,24 @@ define void @prop_byval_readonly(ptr %p) {
559559
call void @foo_byval_readonly(ptr %p)
560560
ret void
561561
}
562+
563+
define ptr @caller_bad_param_prop(ptr %p1, ptr %p2, i64 %x) {
564+
; CHECK-LABEL: define {{[^@]+}}@caller_bad_param_prop
565+
; CHECK-SAME: (ptr [[P1:%.*]], ptr [[P2:%.*]], i64 [[X:%.*]]) {
566+
; CHECK-NEXT: [[TMP1:%.*]] = call ptr [[P1]](i64 [[X]], ptr [[P2]])
567+
; CHECK-NEXT: ret ptr [[TMP1]]
568+
;
569+
%1 = call ptr %p1(i64 %x, ptr %p2)
570+
%2 = call ptr @callee_bad_param_prop(ptr %1)
571+
ret ptr %2
572+
}
573+
574+
define ptr @callee_bad_param_prop(ptr readonly %x) {
575+
; CHECK-LABEL: define {{[^@]+}}@callee_bad_param_prop
576+
; CHECK-SAME: (ptr readonly [[X:%.*]]) {
577+
; CHECK-NEXT: [[R:%.*]] = tail call ptr @llvm.ptrmask.p0.i64(ptr [[X]], i64 -1)
578+
; CHECK-NEXT: ret ptr [[R]]
579+
;
580+
%r = tail call ptr @llvm.ptrmask(ptr %x, i64 -1)
581+
ret ptr %r
582+
}

llvm/test/Transforms/Inline/ret_attr_align_and_noundef.ll

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -421,3 +421,43 @@ define i8 @caller16_not_intersecting_ranges() {
421421
call void @use.val(i8 %r)
422422
ret i8 %r
423423
}
424+
425+
426+
define ptr @caller_bad_ret_prop(ptr %p1, ptr %p2, i64 %x, ptr %other) {
427+
; CHECK-LABEL: define ptr @caller_bad_ret_prop
428+
; CHECK-SAME: (ptr [[P1:%.*]], ptr [[P2:%.*]], i64 [[X:%.*]], ptr [[OTHER:%.*]]) {
429+
; CHECK-NEXT: [[TMP1:%.*]] = call noundef ptr [[P1]](i64 [[X]], ptr [[P2]])
430+
; CHECK-NEXT: [[CMP_I:%.*]] = icmp eq ptr [[TMP1]], null
431+
; CHECK-NEXT: br i1 [[CMP_I]], label [[T_I:%.*]], label [[F_I:%.*]]
432+
; CHECK: T.i:
433+
; CHECK-NEXT: br label [[CALLEE_BAD_RET_PROP_EXIT:%.*]]
434+
; CHECK: F.i:
435+
; CHECK-NEXT: br label [[CALLEE_BAD_RET_PROP_EXIT]]
436+
; CHECK: callee_bad_ret_prop.exit:
437+
; CHECK-NEXT: [[TMP2:%.*]] = phi ptr [ [[OTHER]], [[T_I]] ], [ [[TMP1]], [[F_I]] ]
438+
; CHECK-NEXT: ret ptr [[TMP2]]
439+
;
440+
%1 = call noundef ptr %p1(i64 %x, ptr %p2)
441+
%2 = call nonnull ptr @callee_bad_ret_prop(ptr %1, ptr %other)
442+
ret ptr %2
443+
}
444+
445+
define ptr @callee_bad_ret_prop(ptr %x, ptr %other) {
446+
; CHECK-LABEL: define ptr @callee_bad_ret_prop
447+
; CHECK-SAME: (ptr [[X:%.*]], ptr [[OTHER:%.*]]) {
448+
; CHECK-NEXT: [[CMP:%.*]] = icmp eq ptr [[X]], null
449+
; CHECK-NEXT: br i1 [[CMP]], label [[T:%.*]], label [[F:%.*]]
450+
; CHECK: T:
451+
; CHECK-NEXT: ret ptr [[OTHER]]
452+
; CHECK: F:
453+
; CHECK-NEXT: [[R:%.*]] = tail call ptr @llvm.ptrmask.p0.i64(ptr [[X]], i64 -1)
454+
; CHECK-NEXT: ret ptr [[R]]
455+
;
456+
%cmp = icmp eq ptr %x, null
457+
br i1 %cmp, label %T, label %F
458+
T:
459+
ret ptr %other
460+
F:
461+
%r = tail call ptr @llvm.ptrmask(ptr %x, i64 -1)
462+
ret ptr %r
463+
}

0 commit comments

Comments
 (0)