Skip to content

Commit 3d28947

Browse files
committed
[Inliner] Propagate more attributes to params when inlining
Add support for propagating: - `derefereancable` - `derefereancable_or_null` - `align` - `nonnull` - `nofree` These are only propagated if the parameter to the to-be-inlined callsite match the exact parameter used in the to-be-inlined function.
1 parent 362157f commit 3d28947

File tree

6 files changed

+75
-26
lines changed

6 files changed

+75
-26
lines changed

clang/test/CodeGen/attr-counted-by-pr88931.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ void init(void * __attribute__((pass_dynamic_object_size(0))));
1313
// CHECK-LABEL: define dso_local void @_ZN3foo3barC1Ev(
1414
// CHECK-SAME: ptr noundef nonnull align 4 dereferenceable(1) [[THIS:%.*]]) unnamed_addr #[[ATTR0:[0-9]+]] align 2 {
1515
// CHECK-NEXT: entry:
16-
// CHECK-NEXT: tail call void @_Z4initPvU25pass_dynamic_object_size0(ptr noundef nonnull [[THIS]], i64 noundef -1) #[[ATTR2:[0-9]+]]
16+
// CHECK-NEXT: tail call void @_Z4initPvU25pass_dynamic_object_size0(ptr noundef nonnull align 4 dereferenceable(1) [[THIS]], i64 noundef -1) #[[ATTR2:[0-9]+]]
1717
// CHECK-NEXT: ret void
1818
//
1919
foo::bar::bar() {

clang/test/OpenMP/bug57757.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ void foo() {
3939
// CHECK-NEXT: ]
4040
// CHECK: .untied.jmp..i:
4141
// CHECK-NEXT: store i32 1, ptr [[TMP2]], align 4, !tbaa [[TBAA16]], !alias.scope [[META13]], !noalias [[META17]]
42-
// CHECK-NEXT: [[TMP4:%.*]] = tail call i32 @__kmpc_omp_task(ptr nonnull @[[GLOB1]], i32 [[TMP0]], ptr [[TMP1]]), !noalias [[META13]]
42+
// CHECK-NEXT: [[TMP4:%.*]] = tail call i32 @__kmpc_omp_task(ptr nonnull @[[GLOB1]], i32 [[TMP0]], ptr nonnull [[TMP1]]), !noalias [[META13]]
4343
// CHECK-NEXT: br label [[DOTOMP_OUTLINED__EXIT]]
4444
// CHECK: .untied.next..i:
4545
// CHECK-NEXT: [[TMP5:%.*]] = getelementptr inbounds i8, ptr [[TMP1]], i64 40

llvm/lib/Transforms/Utils/InlineFunction.cpp

Lines changed: 62 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1353,20 +1353,41 @@ static void AddParamAndFnBasicAttributes(const CallBase &CB,
13531353
auto &Context = CalledFunction->getContext();
13541354

13551355
// Collect valid attributes for all params.
1356-
SmallVector<AttrBuilder> ValidParamAttrs;
1356+
SmallVector<AttrBuilder> ValidObjParamAttrs, ValidExactParamAttrs;
13571357
bool HasAttrToPropagate = false;
13581358

13591359
for (unsigned I = 0, E = CB.arg_size(); I < E; ++I) {
1360-
ValidParamAttrs.emplace_back(AttrBuilder{CB.getContext()});
1360+
ValidObjParamAttrs.emplace_back(AttrBuilder{CB.getContext()});
1361+
ValidExactParamAttrs.emplace_back(AttrBuilder{CB.getContext()});
13611362
// Access attributes can be propagated to any param with the same underlying
13621363
// object as the argument.
13631364
if (CB.paramHasAttr(I, Attribute::ReadNone))
1364-
ValidParamAttrs.back().addAttribute(Attribute::ReadNone);
1365+
ValidObjParamAttrs.back().addAttribute(Attribute::ReadNone);
13651366
if (CB.paramHasAttr(I, Attribute::ReadOnly))
1366-
ValidParamAttrs.back().addAttribute(Attribute::ReadOnly);
1367+
ValidObjParamAttrs.back().addAttribute(Attribute::ReadOnly);
13671368
if (CB.paramHasAttr(I, Attribute::WriteOnly))
1368-
ValidParamAttrs.back().addAttribute(Attribute::WriteOnly);
1369-
HasAttrToPropagate |= ValidParamAttrs.back().hasAttributes();
1369+
ValidObjParamAttrs.back().addAttribute(Attribute::WriteOnly);
1370+
1371+
// Attributes we can only propagate if the exact parameter is forwarded.
1372+
1373+
// We can propagate both poison generating and UB generating attributes
1374+
// without any extra checks. The only attribute that is tricky to propagate
1375+
// is `noundef` (skipped for now) as that can create new UB where previous
1376+
// behavior was just using a poison value.
1377+
if (auto DerefBytes = CB.getParamDereferenceableBytes(I))
1378+
ValidExactParamAttrs.back().addDereferenceableAttr(DerefBytes);
1379+
if (auto DerefOrNullBytes = CB.getParamDereferenceableOrNullBytes(I))
1380+
ValidExactParamAttrs.back().addDereferenceableOrNullAttr(
1381+
DerefOrNullBytes);
1382+
if (CB.paramHasAttr(I, Attribute::NoFree))
1383+
ValidExactParamAttrs.back().addAttribute(Attribute::NoFree);
1384+
if (CB.paramHasAttr(I, Attribute::NonNull))
1385+
ValidExactParamAttrs.back().addAttribute(Attribute::NonNull);
1386+
if (auto Align = CB.getParamAlign(I))
1387+
ValidExactParamAttrs.back().addAlignmentAttr(Align);
1388+
1389+
HasAttrToPropagate |= ValidObjParamAttrs.back().hasAttributes();
1390+
HasAttrToPropagate |= ValidExactParamAttrs.back().hasAttributes();
13701391
}
13711392

13721393
// Won't be able to propagate anything.
@@ -1384,21 +1405,49 @@ static void AddParamAndFnBasicAttributes(const CallBase &CB,
13841405
AttributeList AL = NewInnerCB->getAttributes();
13851406
for (unsigned I = 0, E = InnerCB->arg_size(); I < E; ++I) {
13861407
// Check if the underlying value for the parameter is an argument.
1387-
const Value *UnderlyingV =
1388-
getUnderlyingObject(InnerCB->getArgOperand(I));
1389-
const Argument *Arg = dyn_cast<Argument>(UnderlyingV);
1390-
if (!Arg)
1391-
continue;
1408+
const Argument *Arg = dyn_cast<Argument>(InnerCB->getArgOperand(I));
1409+
unsigned ArgNo;
1410+
if (Arg) {
1411+
ArgNo = Arg->getArgNo();
1412+
// For dereferenceable, dereferenceable_or_null, align, etc...
1413+
// we don't want to propagate if the existing param has the same
1414+
// attribute with "better" constraints. So, only remove from the
1415+
// existing AL if the region of the existing param is smaller than
1416+
// what we can propagate. AttributeList's merge API honours the
1417+
// already existing attribute value so we choose the "better"
1418+
// attribute by removing if the existing one is worse.
1419+
if (AL.getParamDereferenceableBytes(I) <
1420+
ValidExactParamAttrs[ArgNo].getDereferenceableBytes())
1421+
AL =
1422+
AL.removeParamAttribute(Context, I, Attribute::Dereferenceable);
1423+
if (AL.getParamDereferenceableOrNullBytes(I) <
1424+
ValidExactParamAttrs[ArgNo].getDereferenceableOrNullBytes())
1425+
AL =
1426+
AL.removeParamAttribute(Context, I, Attribute::Dereferenceable);
1427+
if (AL.getParamAlignment(I).valueOrOne() <
1428+
ValidExactParamAttrs[ArgNo].getAlignment().valueOrOne())
1429+
AL = AL.removeParamAttribute(Context, I, Attribute::Alignment);
1430+
1431+
AL = AL.addParamAttributes(Context, I, ValidExactParamAttrs[ArgNo]);
1432+
1433+
} else {
1434+
// Check if the underlying value for the parameter is an argument.
1435+
const Value *UnderlyingV =
1436+
getUnderlyingObject(InnerCB->getArgOperand(I));
1437+
Arg = dyn_cast<Argument>(UnderlyingV);
1438+
if (!Arg)
1439+
continue;
1440+
ArgNo = Arg->getArgNo();
1441+
}
13921442

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

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

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

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -305,7 +305,7 @@ define void @prop_param_callbase_def_1x_partial_3(ptr %p, ptr %p2) {
305305
define void @prop_deref(ptr %p) {
306306
; CHECK-LABEL: define {{[^@]+}}@prop_deref
307307
; CHECK-SAME: (ptr [[P:%.*]]) {
308-
; CHECK-NEXT: call void @bar1(ptr [[P]])
308+
; CHECK-NEXT: call void @bar1(ptr dereferenceable(16) [[P]])
309309
; CHECK-NEXT: ret void
310310
;
311311
call void @foo1(ptr dereferenceable(16) %p)
@@ -315,7 +315,7 @@ define void @prop_deref(ptr %p) {
315315
define void @prop_deref_or_null(ptr %p) {
316316
; CHECK-LABEL: define {{[^@]+}}@prop_deref_or_null
317317
; CHECK-SAME: (ptr [[P:%.*]]) {
318-
; CHECK-NEXT: call void @bar1(ptr [[P]])
318+
; CHECK-NEXT: call void @bar1(ptr dereferenceable_or_null(256) [[P]])
319319
; CHECK-NEXT: ret void
320320
;
321321
call void @foo1(ptr dereferenceable_or_null(256) %p)
@@ -325,7 +325,7 @@ define void @prop_deref_or_null(ptr %p) {
325325
define void @prop_param_nonnull_and_align(ptr %p) {
326326
; CHECK-LABEL: define {{[^@]+}}@prop_param_nonnull_and_align
327327
; CHECK-SAME: (ptr [[P:%.*]]) {
328-
; CHECK-NEXT: call void @bar1(ptr [[P]])
328+
; CHECK-NEXT: call void @bar1(ptr nonnull align 32 [[P]])
329329
; CHECK-NEXT: ret void
330330
;
331331
call void @foo1(ptr nonnull align 32 %p)
@@ -335,7 +335,7 @@ define void @prop_param_nonnull_and_align(ptr %p) {
335335
define void @prop_param_nofree_and_align(ptr %p) {
336336
; CHECK-LABEL: define {{[^@]+}}@prop_param_nofree_and_align
337337
; CHECK-SAME: (ptr [[P:%.*]]) {
338-
; CHECK-NEXT: call void @bar1(ptr [[P]])
338+
; CHECK-NEXT: call void @bar1(ptr nofree align 32 [[P]])
339339
; CHECK-NEXT: ret void
340340
;
341341
call void @foo1(ptr nofree align 32 %p)
@@ -345,7 +345,7 @@ define void @prop_param_nofree_and_align(ptr %p) {
345345
define void @prop_param_deref_align_no_update(ptr %p) {
346346
; CHECK-LABEL: define {{[^@]+}}@prop_param_deref_align_no_update
347347
; CHECK-SAME: (ptr [[P:%.*]]) {
348-
; CHECK-NEXT: call void @bar1(ptr align 64 dereferenceable(512) [[P]])
348+
; CHECK-NEXT: call void @bar1(ptr align 4 dereferenceable(64) [[P]])
349349
; CHECK-NEXT: ret void
350350
;
351351
call void @foo1_bar_aligned64_deref512(ptr align 4 dereferenceable(64) %p)
@@ -355,7 +355,7 @@ define void @prop_param_deref_align_no_update(ptr %p) {
355355
define void @prop_param_deref_align_update(ptr %p) {
356356
; CHECK-LABEL: define {{[^@]+}}@prop_param_deref_align_update
357357
; CHECK-SAME: (ptr [[P:%.*]]) {
358-
; CHECK-NEXT: call void @bar1(ptr align 64 dereferenceable(512) [[P]])
358+
; CHECK-NEXT: call void @bar1(ptr align 128 dereferenceable(1024) [[P]])
359359
; CHECK-NEXT: ret void
360360
;
361361
call void @foo1_bar_aligned64_deref512(ptr align 128 dereferenceable(1024) %p)
@@ -365,7 +365,7 @@ define void @prop_param_deref_align_update(ptr %p) {
365365
define void @prop_param_deref_or_null_update(ptr %p) {
366366
; CHECK-LABEL: define {{[^@]+}}@prop_param_deref_or_null_update
367367
; CHECK-SAME: (ptr [[P:%.*]]) {
368-
; CHECK-NEXT: call void @bar1(ptr align 512 dereferenceable_or_null(512) [[P]])
368+
; CHECK-NEXT: call void @bar1(ptr align 512 dereferenceable_or_null(1024) [[P]])
369369
; CHECK-NEXT: ret void
370370
;
371371
call void @foo1_bar_aligned512_deref_or_null512(ptr dereferenceable_or_null(1024) %p)
@@ -375,7 +375,7 @@ define void @prop_param_deref_or_null_update(ptr %p) {
375375
define void @prop_param_deref_or_null_no_update(ptr %p) {
376376
; CHECK-LABEL: define {{[^@]+}}@prop_param_deref_or_null_no_update
377377
; CHECK-SAME: (ptr [[P:%.*]]) {
378-
; CHECK-NEXT: call void @bar1(ptr align 512 dereferenceable_or_null(512) [[P]])
378+
; CHECK-NEXT: call void @bar1(ptr align 512 dereferenceable_or_null(32) [[P]])
379379
; CHECK-NEXT: ret void
380380
;
381381
call void @foo1_bar_aligned512_deref_or_null512(ptr dereferenceable_or_null(32) %p)

llvm/test/Transforms/Inline/assumptions-from-callsite-attrs.ll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ declare void @h(ptr %p, ptr %q, ptr %z)
88
define void @f(ptr %p, ptr %q, ptr %z) {
99
; CHECK-LABEL: define void @f
1010
; CHECK-SAME: (ptr [[P:%.*]], ptr [[Q:%.*]], ptr [[Z:%.*]]) {
11-
; CHECK-NEXT: call void @h(ptr [[P]], ptr [[Q]], ptr [[Z]])
11+
; CHECK-NEXT: call void @h(ptr nonnull [[P]], ptr [[Q]], ptr nonnull [[Z]])
1212
; CHECK-NEXT: ret void
1313
;
1414
call void @g(ptr nonnull %p, ptr %q, ptr nonnull %z)

llvm/test/Transforms/Inline/byval.ll

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ define void @test3() nounwind {
106106
; CHECK-NEXT: [[S:%.*]] = alloca [[STRUCT_SS]], align 1
107107
; CHECK-NEXT: call void @llvm.lifetime.start.p0(i64 12, ptr [[S1]])
108108
; CHECK-NEXT: call void @llvm.memcpy.p0.p0.i64(ptr align 1 [[S1]], ptr align 1 [[S]], i64 12, i1 false)
109-
; CHECK-NEXT: call void @g3(ptr [[S1]]) #[[ATTR0]]
109+
; CHECK-NEXT: call void @g3(ptr align 64 [[S1]]) #[[ATTR0]]
110110
; CHECK-NEXT: call void @llvm.lifetime.end.p0(i64 12, ptr [[S1]])
111111
; CHECK-NEXT: ret void
112112
;
@@ -131,7 +131,7 @@ define i32 @test4() nounwind {
131131
; CHECK-SAME: ) #[[ATTR0]] {
132132
; CHECK-NEXT: entry:
133133
; CHECK-NEXT: [[S:%.*]] = alloca [[STRUCT_SS:%.*]], align 64
134-
; CHECK-NEXT: call void @g3(ptr [[S]]) #[[ATTR0]]
134+
; CHECK-NEXT: call void @g3(ptr align 64 [[S]]) #[[ATTR0]]
135135
; CHECK-NEXT: ret i32 4
136136
;
137137
entry:

0 commit comments

Comments
 (0)