Skip to content

Commit ae778ae

Browse files
authored
[Inliner] Propagate more attributes to params when inlining (#91101)
- **[Inliner] Add tests for propagating more parameter attributes; NFC** - **[Inliner] Propagate more attributes to params when inlining** Add support for propagating: - `derefereancable` - `derefereancable_or_null` - `align` - `nonnull` - `range` 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 0850e72 commit ae778ae

File tree

9 files changed

+259
-29
lines changed

9 files changed

+259
-29
lines changed

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

+1-1
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

+1-1
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 nuw i8, ptr [[TMP1]], i64 40

llvm/include/llvm/IR/Attributes.h

+7
Original file line numberDiff line numberDiff line change
@@ -947,6 +947,9 @@ class AttributeList {
947947
/// arg.
948948
uint64_t getParamDereferenceableOrNullBytes(unsigned ArgNo) const;
949949

950+
/// Get range (or std::nullopt if unknown) of an arg.
951+
std::optional<ConstantRange> getParamRange(unsigned ArgNo) const;
952+
950953
/// Get the disallowed floating-point classes of the return value.
951954
FPClassTest getRetNoFPClass() const;
952955

@@ -1123,6 +1126,10 @@ class AttrBuilder {
11231126
/// invalid if the Kind is not present in the builder.
11241127
Attribute getAttribute(StringRef Kind) const;
11251128

1129+
/// Retrieve the range if the attribute exists (std::nullopt is returned
1130+
/// otherwise).
1131+
std::optional<ConstantRange> getRange() const;
1132+
11261133
/// Return raw (possibly packed/encoded) value of integer attribute or
11271134
/// std::nullopt if not set.
11281135
std::optional<uint64_t> getRawIntAttr(Attribute::AttrKind Kind) const;

llvm/lib/IR/Attributes.cpp

+15
Original file line numberDiff line numberDiff line change
@@ -1931,6 +1931,14 @@ AttributeList::getParamDereferenceableOrNullBytes(unsigned Index) const {
19311931
return getParamAttrs(Index).getDereferenceableOrNullBytes();
19321932
}
19331933

1934+
std::optional<ConstantRange>
1935+
AttributeList::getParamRange(unsigned ArgNo) const {
1936+
auto RangeAttr = getParamAttrs(ArgNo).getAttribute(Attribute::Range);
1937+
if (RangeAttr.isValid())
1938+
return RangeAttr.getRange();
1939+
return std::nullopt;
1940+
}
1941+
19341942
FPClassTest AttributeList::getRetNoFPClass() const {
19351943
return getRetAttrs().getNoFPClass();
19361944
}
@@ -2277,6 +2285,13 @@ Attribute AttrBuilder::getAttribute(StringRef A) const {
22772285
return {};
22782286
}
22792287

2288+
std::optional<ConstantRange> AttrBuilder::getRange() const {
2289+
const Attribute RangeAttr = getAttribute(Attribute::Range);
2290+
if (RangeAttr.isValid())
2291+
return RangeAttr.getRange();
2292+
return std::nullopt;
2293+
}
2294+
22802295
bool AttrBuilder::contains(Attribute::AttrKind A) const {
22812296
return getAttribute(A).isValid();
22822297
}

llvm/lib/Transforms/Utils/InlineFunction.cpp

+74-16
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
#include "llvm/Analysis/VectorUtils.h"
3535
#include "llvm/IR/Argument.h"
3636
#include "llvm/IR/AttributeMask.h"
37+
#include "llvm/IR/Attributes.h"
3738
#include "llvm/IR/BasicBlock.h"
3839
#include "llvm/IR/CFG.h"
3940
#include "llvm/IR/Constant.h"
@@ -59,6 +60,7 @@
5960
#include "llvm/IR/MDBuilder.h"
6061
#include "llvm/IR/Metadata.h"
6162
#include "llvm/IR/Module.h"
63+
#include "llvm/IR/PatternMatch.h"
6264
#include "llvm/IR/ProfDataUtils.h"
6365
#include "llvm/IR/Type.h"
6466
#include "llvm/IR/User.h"
@@ -1358,18 +1360,36 @@ static void AddParamAndFnBasicAttributes(const CallBase &CB,
13581360
auto &Context = CalledFunction->getContext();
13591361

13601362
// Collect valid attributes for all params.
1361-
SmallVector<AttrBuilder> ValidParamAttrs;
1363+
SmallVector<AttrBuilder> ValidObjParamAttrs, ValidExactParamAttrs;
13621364
bool HasAttrToPropagate = false;
13631365

1366+
// Attributes we can only propagate if the exact parameter is forwarded.
1367+
// We can propagate both poison generating and UB generating attributes
1368+
// without any extra checks. The only attribute that is tricky to propagate
1369+
// is `noundef` (skipped for now) as that can create new UB where previous
1370+
// behavior was just using a poison value.
1371+
static const Attribute::AttrKind ExactAttrsToPropagate[] = {
1372+
Attribute::Dereferenceable, Attribute::DereferenceableOrNull,
1373+
Attribute::NonNull, Attribute::Alignment, Attribute::Range};
1374+
13641375
for (unsigned I = 0, E = CB.arg_size(); I < E; ++I) {
1365-
ValidParamAttrs.emplace_back(AttrBuilder{CB.getContext()});
1376+
ValidObjParamAttrs.emplace_back(AttrBuilder{CB.getContext()});
1377+
ValidExactParamAttrs.emplace_back(AttrBuilder{CB.getContext()});
13661378
// Access attributes can be propagated to any param with the same underlying
13671379
// object as the argument.
13681380
if (CB.paramHasAttr(I, Attribute::ReadNone))
1369-
ValidParamAttrs.back().addAttribute(Attribute::ReadNone);
1381+
ValidObjParamAttrs.back().addAttribute(Attribute::ReadNone);
13701382
if (CB.paramHasAttr(I, Attribute::ReadOnly))
1371-
ValidParamAttrs.back().addAttribute(Attribute::ReadOnly);
1372-
HasAttrToPropagate |= ValidParamAttrs.back().hasAttributes();
1383+
ValidObjParamAttrs.back().addAttribute(Attribute::ReadOnly);
1384+
1385+
for (Attribute::AttrKind AK : ExactAttrsToPropagate) {
1386+
Attribute Attr = CB.getParamAttr(I, AK);
1387+
if (Attr.isValid())
1388+
ValidExactParamAttrs.back().addAttribute(Attr);
1389+
}
1390+
1391+
HasAttrToPropagate |= ValidObjParamAttrs.back().hasAttributes();
1392+
HasAttrToPropagate |= ValidExactParamAttrs.back().hasAttributes();
13731393
}
13741394

13751395
// Won't be able to propagate anything.
@@ -1391,22 +1411,60 @@ static void AddParamAndFnBasicAttributes(const CallBase &CB,
13911411

13921412
AttributeList AL = NewInnerCB->getAttributes();
13931413
for (unsigned I = 0, E = InnerCB->arg_size(); I < E; ++I) {
1394-
// Check if the underlying value for the parameter is an argument.
1395-
const Value *UnderlyingV =
1396-
getUnderlyingObject(InnerCB->getArgOperand(I));
1397-
const Argument *Arg = dyn_cast<Argument>(UnderlyingV);
1398-
if (!Arg)
1414+
// It's unsound or requires special handling to propagate
1415+
// attributes to byval arguments. Even if CalledFunction
1416+
// doesn't e.g. write to the argument (readonly), the call to
1417+
// NewInnerCB may write to its by-value copy.
1418+
if (NewInnerCB->paramHasAttr(I, Attribute::ByVal))
13991419
continue;
14001420

1401-
if (NewInnerCB->paramHasAttr(I, Attribute::ByVal))
1402-
// It's unsound to propagate memory attributes to byval arguments.
1403-
// Even if CalledFunction doesn't e.g. write to the argument,
1404-
// the call to NewInnerCB may write to its by-value copy.
1421+
// Don't bother propagating attrs to constants.
1422+
if (match(NewInnerCB->getArgOperand(I),
1423+
llvm::PatternMatch::m_ImmConstant()))
14051424
continue;
14061425

1407-
unsigned ArgNo = Arg->getArgNo();
1426+
// Check if the underlying value for the parameter is an argument.
1427+
const Argument *Arg = dyn_cast<Argument>(InnerCB->getArgOperand(I));
1428+
unsigned ArgNo;
1429+
if (Arg) {
1430+
ArgNo = Arg->getArgNo();
1431+
// For dereferenceable, dereferenceable_or_null, align, etc...
1432+
// we don't want to propagate if the existing param has the same
1433+
// attribute with "better" constraints. So remove from the
1434+
// new AL if the region of the existing param is larger than
1435+
// what we can propagate.
1436+
AttrBuilder NewAB{
1437+
Context, AttributeSet::get(Context, ValidExactParamAttrs[ArgNo])};
1438+
if (AL.getParamDereferenceableBytes(I) >
1439+
NewAB.getDereferenceableBytes())
1440+
NewAB.removeAttribute(Attribute::Dereferenceable);
1441+
if (AL.getParamDereferenceableOrNullBytes(I) >
1442+
NewAB.getDereferenceableOrNullBytes())
1443+
NewAB.removeAttribute(Attribute::DereferenceableOrNull);
1444+
if (AL.getParamAlignment(I).valueOrOne() >
1445+
NewAB.getAlignment().valueOrOne())
1446+
NewAB.removeAttribute(Attribute::Alignment);
1447+
if (auto ExistingRange = AL.getParamRange(I)) {
1448+
if (auto NewRange = NewAB.getRange()) {
1449+
ConstantRange CombinedRange =
1450+
ExistingRange->intersectWith(*NewRange);
1451+
NewAB.removeAttribute(Attribute::Range);
1452+
NewAB.addRangeAttr(CombinedRange);
1453+
}
1454+
}
1455+
AL = AL.addParamAttributes(Context, I, NewAB);
1456+
} else {
1457+
// Check if the underlying value for the parameter is an argument.
1458+
const Value *UnderlyingV =
1459+
getUnderlyingObject(InnerCB->getArgOperand(I));
1460+
Arg = dyn_cast<Argument>(UnderlyingV);
1461+
if (!Arg)
1462+
continue;
1463+
ArgNo = Arg->getArgNo();
1464+
}
1465+
14081466
// If so, propagate its access attributes.
1409-
AL = AL.addParamAttributes(Context, I, ValidParamAttrs[ArgNo]);
1467+
AL = AL.addParamAttributes(Context, I, ValidObjParamAttrs[ArgNo]);
14101468
// We can have conflicting attributes from the inner callsite and
14111469
// to-be-inlined callsite. In that case, choose the most
14121470
// restrictive.

0 commit comments

Comments
 (0)