Skip to content

Commit 5d41c20

Browse files
goldsteinntru
authored andcommitted
[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. (cherry picked from commit a9352a0)
1 parent 38934af commit 5d41c20

File tree

3 files changed

+88
-4
lines changed

3 files changed

+88
-4
lines changed

llvm/lib/Transforms/Utils/InlineFunction.cpp

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

@@ -1380,6 +1381,11 @@ static void AddParamAndFnBasicAttributes(const CallBase &CB,
13801381
auto *NewInnerCB = dyn_cast_or_null<CallBase>(VMap.lookup(InnerCB));
13811382
if (!NewInnerCB)
13821383
continue;
1384+
// The InnerCB might have be simplified during the inlining
1385+
// process which can make propagation incorrect.
1386+
if (InlinedFunctionInfo.isSimplified(InnerCB, NewInnerCB))
1387+
continue;
1388+
13831389
AttributeList AL = NewInnerCB->getAttributes();
13841390
for (unsigned I = 0, E = InnerCB->arg_size(); I < E; ++I) {
13851391
// Check if the underlying value for the parameter is an argument.
@@ -1455,7 +1461,8 @@ static AttrBuilder IdentifyValidPoisonGeneratingAttributes(CallBase &CB) {
14551461
return Valid;
14561462
}
14571463

1458-
static void AddReturnAttributes(CallBase &CB, ValueToValueMapTy &VMap) {
1464+
static void AddReturnAttributes(CallBase &CB, ValueToValueMapTy &VMap,
1465+
ClonedCodeInfo &InlinedFunctionInfo) {
14591466
AttrBuilder ValidUB = IdentifyValidUBGeneratingAttributes(CB);
14601467
AttrBuilder ValidPG = IdentifyValidPoisonGeneratingAttributes(CB);
14611468
if (!ValidUB.hasAttributes() && !ValidPG.hasAttributes())
@@ -1474,6 +1481,11 @@ static void AddReturnAttributes(CallBase &CB, ValueToValueMapTy &VMap) {
14741481
auto *NewRetVal = dyn_cast_or_null<CallBase>(VMap.lookup(RetVal));
14751482
if (!NewRetVal)
14761483
continue;
1484+
1485+
// The RetVal might have be simplified during the inlining
1486+
// process which can make propagation incorrect.
1487+
if (InlinedFunctionInfo.isSimplified(RetVal, NewRetVal))
1488+
continue;
14771489
// Backward propagation of attributes to the returned value may be incorrect
14781490
// if it is control flow dependent.
14791491
// Consider:
@@ -2456,11 +2468,11 @@ llvm::InlineResult llvm::InlineFunction(CallBase &CB, InlineFunctionInfo &IFI,
24562468

24572469
// Clone return attributes on the callsite into the calls within the inlined
24582470
// function which feed into its return value.
2459-
AddReturnAttributes(CB, VMap);
2471+
AddReturnAttributes(CB, VMap, InlinedFunctionInfo);
24602472

24612473
// Clone attributes on the params of the callsite to calls within the
24622474
// inlined function which use the same param.
2463-
AddParamAndFnBasicAttributes(CB, VMap);
2475+
AddParamAndFnBasicAttributes(CB, VMap, InlinedFunctionInfo);
24642476

24652477
propagateMemProfMetadata(CalledFunc, CB,
24662478
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: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -410,3 +410,54 @@ define i8 @caller15_okay_intersect_ranges() {
410410
call void @use.val(i8 %r)
411411
ret i8 %r
412412
}
413+
414+
define i8 @caller16_not_intersecting_ranges() {
415+
; CHECK-LABEL: define i8 @caller16_not_intersecting_ranges() {
416+
; CHECK-NEXT: [[R_I:%.*]] = call range(i8 0, 0) i8 @val8()
417+
; CHECK-NEXT: call void @use.val(i8 [[R_I]])
418+
; CHECK-NEXT: ret i8 [[R_I]]
419+
;
420+
%r = call range(i8 0, 5) i8 @callee15()
421+
call void @use.val(i8 %r)
422+
ret i8 %r
423+
}
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)