[SimplifyCFG] Supporting hoisting/sinking callbases with differing attrs #109472
+607
−21
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Some (many) attributes can safely be dropped to enable sinking. For example removing `nonnull` on a return/param can't affect correctness.
|
@llvm/pr-subscribers-llvm-transforms @llvm/pr-subscribers-llvm-ir Author: None (goldsteinn) Changes
Patch is 33.70 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/109472.diff 7 Files Affected:
diff --git a/llvm/include/llvm/IR/Attributes.h b/llvm/include/llvm/IR/Attributes.h
index 5a80a072dbbd27..061ecdf59d0464 100644
--- a/llvm/include/llvm/IR/Attributes.h
+++ b/llvm/include/llvm/IR/Attributes.h
@@ -208,8 +208,15 @@ class Attribute {
/// Return true if the target-dependent attribute is present.
bool hasAttribute(StringRef Val) const;
+ /// Returns true if the attribute's kind can be represented as an enum (Enum,
+ /// Integer, Type, ConstantRange, or ConstantRangeList attribute).
+ bool hasKindAsEnum() const {
+ return isEnumAttribute() || isIntAttribute() || isTypeAttribute() ||
+ isConstantRangeAttribute() || isConstantRangeListAttribute();
+ }
+
/// Return the attribute's kind as an enum (Attribute::AttrKind). This
- /// requires the attribute to be an enum, integer, or type attribute.
+ /// requires the attribute be representable as an enum (see: `hasKindAsEnum`).
Attribute::AttrKind getKindAsEnum() const;
/// Return the attribute's value as an integer. This requires that the
diff --git a/llvm/include/llvm/IR/Instruction.h b/llvm/include/llvm/IR/Instruction.h
index 4a1e66a90831c8..478a8d77ea4d93 100644
--- a/llvm/include/llvm/IR/Instruction.h
+++ b/llvm/include/llvm/IR/Instruction.h
@@ -881,16 +881,19 @@ class Instruction : public User,
/// This is like isIdenticalTo, except that it ignores the
/// SubclassOptionalData flags, which may specify conditions under which the
/// instruction's result is undefined.
- bool isIdenticalToWhenDefined(const Instruction *I) const LLVM_READONLY;
+ bool isIdenticalToWhenDefined(const Instruction *I,
+ bool IgnoreAttrs = false) const LLVM_READONLY;
/// When checking for operation equivalence (using isSameOperationAs) it is
/// sometimes useful to ignore certain attributes.
enum OperationEquivalenceFlags {
/// Check for equivalence ignoring load/store alignment.
- CompareIgnoringAlignment = 1<<0,
+ CompareIgnoringAlignment = 1 << 0,
/// Check for equivalence treating a type and a vector of that type
/// as equivalent.
- CompareUsingScalarTypes = 1<<1
+ CompareUsingScalarTypes = 1 << 1,
+ /// Check for equivalence ignoring callbase attrs.
+ CompareIgnoringAttrs = 1 << 2,
};
/// This function determines if the specified instruction executes the same
@@ -911,8 +914,8 @@ class Instruction : public User,
/// @returns true if the specific instruction has the same opcde specific
/// characteristics as the current one. Determine if one instruction has the
/// same state as another.
- bool hasSameSpecialState(const Instruction *I2,
- bool IgnoreAlignment = false) const LLVM_READONLY;
+ bool hasSameSpecialState(const Instruction *I2, bool IgnoreAlignment = false,
+ bool IgnoreAttrs = false) const LLVM_READONLY;
/// Return true if there are any uses of this instruction in blocks other than
/// the specified block. Note that PHI nodes are considered to evaluate their
diff --git a/llvm/lib/IR/Attributes.cpp b/llvm/lib/IR/Attributes.cpp
index fa124e46483dce..061c724fd4bd8f 100644
--- a/llvm/lib/IR/Attributes.cpp
+++ b/llvm/lib/IR/Attributes.cpp
@@ -362,8 +362,7 @@ bool Attribute::isConstantRangeListAttribute() const {
Attribute::AttrKind Attribute::getKindAsEnum() const {
if (!pImpl) return None;
- assert((isEnumAttribute() || isIntAttribute() || isTypeAttribute() ||
- isConstantRangeAttribute() || isConstantRangeListAttribute()) &&
+ assert(hasKindAsEnum() &&
"Invalid attribute type to get the kind as an enum!");
return pImpl->getKindAsEnum();
}
diff --git a/llvm/lib/IR/Instruction.cpp b/llvm/lib/IR/Instruction.cpp
index b1c2b0200c8269..07c0041a8dc5ff 100644
--- a/llvm/lib/IR/Instruction.cpp
+++ b/llvm/lib/IR/Instruction.cpp
@@ -785,7 +785,8 @@ const char *Instruction::getOpcodeName(unsigned OpCode) {
/// This must be kept in sync with FunctionComparator::cmpOperations in
/// lib/Transforms/IPO/MergeFunctions.cpp.
bool Instruction::hasSameSpecialState(const Instruction *I2,
- bool IgnoreAlignment) const {
+ bool IgnoreAlignment,
+ bool IgnoreAttrs) const {
auto I1 = this;
assert(I1->getOpcode() == I2->getOpcode() &&
"Can not compare special state of different instructions");
@@ -811,15 +812,18 @@ bool Instruction::hasSameSpecialState(const Instruction *I2,
if (const CallInst *CI = dyn_cast<CallInst>(I1))
return CI->isTailCall() == cast<CallInst>(I2)->isTailCall() &&
CI->getCallingConv() == cast<CallInst>(I2)->getCallingConv() &&
- CI->getAttributes() == cast<CallInst>(I2)->getAttributes() &&
+ (IgnoreAttrs ||
+ CI->getAttributes() == cast<CallInst>(I2)->getAttributes()) &&
CI->hasIdenticalOperandBundleSchema(*cast<CallInst>(I2));
if (const InvokeInst *CI = dyn_cast<InvokeInst>(I1))
return CI->getCallingConv() == cast<InvokeInst>(I2)->getCallingConv() &&
- CI->getAttributes() == cast<InvokeInst>(I2)->getAttributes() &&
+ (IgnoreAttrs ||
+ CI->getAttributes() == cast<InvokeInst>(I2)->getAttributes()) &&
CI->hasIdenticalOperandBundleSchema(*cast<InvokeInst>(I2));
if (const CallBrInst *CI = dyn_cast<CallBrInst>(I1))
return CI->getCallingConv() == cast<CallBrInst>(I2)->getCallingConv() &&
- CI->getAttributes() == cast<CallBrInst>(I2)->getAttributes() &&
+ (IgnoreAttrs ||
+ CI->getAttributes() == cast<CallBrInst>(I2)->getAttributes()) &&
CI->hasIdenticalOperandBundleSchema(*cast<CallBrInst>(I2));
if (const InsertValueInst *IVI = dyn_cast<InsertValueInst>(I1))
return IVI->getIndices() == cast<InsertValueInst>(I2)->getIndices();
@@ -857,10 +861,10 @@ bool Instruction::isIdenticalTo(const Instruction *I) const {
SubclassOptionalData == I->SubclassOptionalData;
}
-bool Instruction::isIdenticalToWhenDefined(const Instruction *I) const {
+bool Instruction::isIdenticalToWhenDefined(const Instruction *I,
+ bool IgnoreAttrs) const {
if (getOpcode() != I->getOpcode() ||
- getNumOperands() != I->getNumOperands() ||
- getType() != I->getType())
+ getNumOperands() != I->getNumOperands() || getType() != I->getType())
return false;
// If both instructions have no operands, they are identical.
@@ -879,7 +883,7 @@ bool Instruction::isIdenticalToWhenDefined(const Instruction *I) const {
otherPHI->block_begin());
}
- return this->hasSameSpecialState(I);
+ return this->hasSameSpecialState(I, /*IgnoreAlignment=*/false, IgnoreAttrs);
}
// Keep this in sync with FunctionComparator::cmpOperations in
@@ -887,7 +891,8 @@ bool Instruction::isIdenticalToWhenDefined(const Instruction *I) const {
bool Instruction::isSameOperationAs(const Instruction *I,
unsigned flags) const {
bool IgnoreAlignment = flags & CompareIgnoringAlignment;
- bool UseScalarTypes = flags & CompareUsingScalarTypes;
+ bool UseScalarTypes = flags & CompareUsingScalarTypes;
+ bool IgnoreAttrs = flags & CompareIgnoringAttrs;
if (getOpcode() != I->getOpcode() ||
getNumOperands() != I->getNumOperands() ||
@@ -905,7 +910,7 @@ bool Instruction::isSameOperationAs(const Instruction *I,
getOperand(i)->getType() != I->getOperand(i)->getType())
return false;
- return this->hasSameSpecialState(I, IgnoreAlignment);
+ return this->hasSameSpecialState(I, IgnoreAlignment, IgnoreAttrs);
}
bool Instruction::isUsedOutsideOfBlock(const BasicBlock *BB) const {
diff --git a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
index 69c4475a494cbe..cd17dc454fd1ad 100644
--- a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
+++ b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
@@ -1591,10 +1591,149 @@ static void hoistLockstepIdenticalDbgVariableRecords(
}
}
+// See if we can intersect the attributes for two callbases (used for sinking).
+static std::optional<AttributeList> tryIntersectAttrs(const CallBase *CB0,
+ const CallBase *CB1) {
+ assert(CB0->getCalledFunction() == CB1->getCalledFunction() &&
+ "Merging attrs for different functions!");
+
+ AttributeList AL0 = CB0->getAttributes();
+ AttributeList AL1 = CB1->getAttributes();
+
+ // Trivial case if attributes match
+ if (AL0 == AL1)
+ return AL0;
+
+ // Otherwise go through all attributes present and make sure they either match
+ // or that dropping them is okay.
+ // Note: At the moment the logic is only concerned with correctness (i.e we
+ // can't sink a callbase with a `ByVal` attr on a param with one that doesn't
+ // have the attr). But there may be some attributes that are not preferable to
+ // drop i.e a certain Range attr might trivialize inlining so intersecting it
+ // with a callbase without the attr might not be profitable.
+ LLVMContext &Ctx = CB0->getContext();
+ auto IntersectAttrs = [&Ctx](AttributeSet AS0,
+ AttributeSet AS1) -> std::optional<AttrBuilder> {
+ AttrBuilder Intersected(Ctx);
+
+ AttributeSet AllAttrs = AS0.addAttributes(Ctx, AS1);
+ for (Attribute Attr : AllAttrs) {
+ if (!Attr.isValid())
+ return std::nullopt;
+
+ // Only supporting enum attrs for now.
+ if (!Attr.hasKindAsEnum())
+ return std::nullopt;
+
+ Attribute::AttrKind Kind = Attr.getKindAsEnum();
+ bool BothContain = AS0.hasAttribute(Kind) && AS1.hasAttribute(Kind);
+ switch (Kind) {
+ default:
+ // Except for the below attrs we know we can intersect safely, fail if
+ // the attributes don't match.
+ if (!BothContain)
+ return std::nullopt;
+ if (AS0.getAttribute(Kind) != AS1.getAttribute(Kind))
+ return std::nullopt;
+ Intersected.addAttribute(Attr);
+ break;
+ // Attributes that can safely be intersected and can safely be thrown
+ // away.
+ case Attribute::Cold:
+ case Attribute::Hot:
+ case Attribute::MustProgress:
+ case Attribute::NoAlias:
+ case Attribute::NoCallback:
+ case Attribute::NoCapture:
+ case Attribute::NoFree:
+ case Attribute::NoRecurse:
+ case Attribute::NoReturn:
+ case Attribute::NoSync:
+ case Attribute::NoUndef:
+ case Attribute::NoUnwind:
+ case Attribute::NonNull:
+ case Attribute::OptimizeForSize:
+ // TODO: We could merge ReadNone + Readonly -> ReadOnly
+ case Attribute::ReadNone:
+ case Attribute::ReadOnly:
+ case Attribute::Returned:
+ case Attribute::Speculatable:
+ case Attribute::WillReturn:
+ case Attribute::Writable:
+ case Attribute::WriteOnly:
+ if (BothContain)
+ Intersected.addAttribute(Attr);
+ break;
+ // Alignment/Dereferenceable/DereferenceableOrNull/Memory/Range we can
+ // safely throw out, but intersection requires us to compare the values
+ // at hand.
+ case Attribute::Alignment:
+ if (BothContain)
+ Intersected.addAlignmentAttr(
+ std::min(AS0.getAlignment().valueOrOne(),
+ AS1.getAlignment().valueOrOne()));
+ break;
+ case Attribute::Dereferenceable:
+ if (BothContain)
+ Intersected.addDereferenceableAttr(std::min(
+ AS0.getDereferenceableBytes(), AS1.getDereferenceableBytes()));
+ break;
+ case Attribute::DereferenceableOrNull:
+ if (BothContain)
+ Intersected.addDereferenceableOrNullAttr(
+ std::min(AS0.getDereferenceableOrNullBytes(),
+ AS1.getDereferenceableOrNullBytes()));
+ break;
+ case Attribute::Memory:
+ if (BothContain)
+ Intersected.addMemoryAttr(AS0.getMemoryEffects() |
+ AS1.getMemoryEffects());
+ break;
+ case Attribute::Range:
+ if (BothContain) {
+ ConstantRange Range0 = AS0.getAttribute(Attribute::Range).getRange();
+ ConstantRange Range1 = AS1.getAttribute(Attribute::Range).getRange();
+ ConstantRange NewRange = Range0.unionWith(Range1);
+ if (!NewRange.isFullSet())
+ Intersected.addRangeAttr(NewRange);
+ }
+ }
+ }
+ return Intersected;
+ };
+
+ // Intersect all attribute types (ret/fn/param).
+ AttributeList IntersectedAttrs{};
+ auto IntersectedRetAttrs =
+ IntersectAttrs(AL0.getRetAttrs(), AL1.getRetAttrs());
+ if (!IntersectedRetAttrs)
+ return std::nullopt;
+ IntersectedAttrs =
+ IntersectedAttrs.addRetAttributes(Ctx, *IntersectedRetAttrs);
+
+ auto IntersectedFnAttrs = IntersectAttrs(AL0.getFnAttrs(), AL1.getFnAttrs());
+ if (!IntersectedFnAttrs)
+ return std::nullopt;
+ IntersectedAttrs = IntersectedAttrs.addFnAttributes(Ctx, *IntersectedFnAttrs);
+
+ for (unsigned ParamIdx = 0; ParamIdx < CB0->arg_size(); ++ParamIdx) {
+ auto IntersectedParamAttrs = IntersectAttrs(AL0.getParamAttrs(ParamIdx),
+ AL1.getParamAttrs(ParamIdx));
+ if (!IntersectedParamAttrs)
+ return std::nullopt;
+ IntersectedAttrs = IntersectedAttrs.addParamAttributes(
+ Ctx, ParamIdx, *IntersectedParamAttrs);
+ }
+ return IntersectedAttrs;
+}
+
static bool areIdenticalUpToCommutativity(const Instruction *I1,
const Instruction *I2) {
- if (I1->isIdenticalToWhenDefined(I2))
+ if (I1->isIdenticalToWhenDefined(I2, /*IgnoreAttrs=*/true)) {
+ if (auto *CB1 = dyn_cast<CallBase>(I1))
+ return tryIntersectAttrs(CB1, cast<CallBase>(I2)).has_value();
return true;
+ }
if (auto *Cmp1 = dyn_cast<CmpInst>(I1))
if (auto *Cmp2 = dyn_cast<CmpInst>(I2))
@@ -1775,6 +1914,14 @@ bool SimplifyCFGOpt::hoistCommonCodeFromSuccessors(BasicBlock *BB,
if (!I2->use_empty())
I2->replaceAllUsesWith(I1);
I1->andIRFlags(I2);
+ if (auto *CB = dyn_cast<CallBase>(I1)) {
+ auto IntersectedAttrs = tryIntersectAttrs(CB, cast<CallBase>(I2));
+ assert(IntersectedAttrs &&
+ "We should not be trying to hoist callbases "
+ "with non-intersectable attributes");
+ CB->setAttributes(*IntersectedAttrs);
+ }
+
combineMetadataForCSE(I1, I2, true);
// I1 and I2 are being combined into a single instruction. Its debug
// location is the merged locations of the original instructions.
@@ -1995,7 +2142,7 @@ static bool canSinkInstructions(
const Instruction *I0 = Insts.front();
const auto I0MMRA = MMRAMetadata(*I0);
for (auto *I : Insts) {
- if (!I->isSameOperationAs(I0))
+ if (!I->isSameOperationAs(I0, Instruction::CompareIgnoringAttrs))
return false;
// swifterror pointers can only be used by a load or store; sinking a load
@@ -2029,7 +2176,7 @@ static bool canSinkInstructions(
// I.e. if we have two direct calls to different callees, we don't want to
// turn that into an indirect call. Likewise, if we have an indirect call,
// and a direct call, we don't actually want to have a single indirect call.
- if (isa<CallBase>(I0)) {
+ if (auto *CB = dyn_cast<CallBase>(I0)) {
auto IsIndirectCall = [](const Instruction *I) {
return cast<CallBase>(I)->isIndirectCall();
};
@@ -2048,6 +2195,11 @@ static bool canSinkInstructions(
else if (Callee != CurrCallee)
return false;
}
+ }
+ // Check that we can intersect the attributes if we sink.
+ for (const Instruction *I : Insts) {
+ if (I != I0 && !tryIntersectAttrs(CB, cast<CallBase>(I)))
+ return false;
}
}
@@ -2152,6 +2304,12 @@ static void sinkLastInstruction(ArrayRef<BasicBlock*> Blocks) {
I0->applyMergedLocation(I0->getDebugLoc(), I->getDebugLoc());
combineMetadataForCSE(I0, I, true);
I0->andIRFlags(I);
+ if (auto *CB = dyn_cast<CallBase>(I0)) {
+ auto IntersectedAttrs = tryIntersectAttrs(CB, cast<CallBase>(I));
+ assert(IntersectedAttrs && "We should not be trying to sink callbases "
+ "with non-intersectable attributes");
+ CB->setAttributes(*IntersectedAttrs);
+ }
}
for (User *U : make_early_inc_range(I0->users())) {
diff --git a/llvm/test/Transforms/SimplifyCFG/hoist-cb-diff-attrs.ll b/llvm/test/Transforms/SimplifyCFG/hoist-cb-diff-attrs.ll
new file mode 100644
index 00000000000000..586792ea76e374
--- /dev/null
+++ b/llvm/test/Transforms/SimplifyCFG/hoist-cb-diff-attrs.ll
@@ -0,0 +1,195 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --function-signature --check-attributes
+; RUN: opt < %s -passes='simplifycfg<hoist-common-insts>' -simplifycfg-require-and-preserve-domtree=1 -S | FileCheck %s
+
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-i128:128-f80:128-n8:16:32:64-S128"
+
+declare ptr @foo(ptr %p, i64 %x)
+declare void @side.effect()
+
+define ptr @test_hoist_int_attrs(i1 %c, ptr %p, i64 %x) {
+; CHECK-LABEL: define {{[^@]+}}@test_hoist_int_attrs
+; CHECK-SAME: (i1 [[C:%.*]], ptr [[P:%.*]], i64 [[X:%.*]]) {
+; CHECK-NEXT: [[R:%.*]] = call ptr @foo(ptr align 32 dereferenceable(50) dereferenceable_or_null(100) [[P]], i64 range(i64 10, 100000) [[X]]) #[[ATTR0:[0-9]+]]
+; CHECK-NEXT: br i1 [[C]], label [[COMMON_RET:%.*]], label [[ELSE:%.*]]
+; CHECK: common.ret:
+; CHECK-NEXT: ret ptr [[R]]
+; CHECK: else:
+; CHECK-NEXT: call void @side.effect()
+; CHECK-NEXT: br label [[COMMON_RET]]
+;
+ br i1 %c, label %if, label %else
+if:
+ %r = call ptr @foo(ptr dereferenceable(50) align 64 dereferenceable_or_null(100) %p, i64 range(i64 10, 1000) %x) memory(read)
+ ret ptr %r
+
+else:
+ %r2 = call ptr @foo(ptr dereferenceable(100) align 32 dereferenceable_or_null(200) %p, i64 range(i64 10000, 100000) %x) memory(write)
+ call void @side.effect()
+ ret ptr %r2
+}
+
+define ptr @test_hoist_int_attrs2(i1 %c, ptr %p, i64 %x) {
+; CHECK-LABEL: define {{[^@]+}}@test_hoist_int_attrs2
+; CHECK-SAME: (i1 [[C:%.*]], ptr [[P:%.*]], i64 [[X:%.*]]) {
+; CHECK-NEXT: [[R:%.*]] = call ptr @foo(ptr dereferenceable(50) [[P]], i64 range(i64 10, 1000) [[X]]) #[[ATTR1:[0-9]+]]
+; CHECK-NEXT: br i1 [[C]], label [[COMMON_RET:%.*]], label [[ELSE:%.*]]
+; CHECK: common.ret:
+; CHECK-NEXT: ret ptr [[R]]
+; CHECK: else:
+; CHECK-NEXT: call void @side.effect()
+; CHECK-NEXT: br label [[COMMON_RET]]
+;
+ br i1 %c, label %if, label %else
+if:
+ %r = call ptr @foo(ptr dereferenceable(50) %p, i64 range(i64 10, 1000) %x) memory(read)
+ ret ptr %r
+
+else:
+ %r2 = call ptr @foo(ptr dereferenceable(100) align 32 dereferenceable_or_null(200) %p, i64 range(i64 11, 100) %x) memory(none)
+ call void @side.effect()
+ ret ptr %r2
+}
+
+define ptr @test_hoist_bool_attrs2(i1 %c, ptr %p, i64 %x) {
+; CHECK-LABEL: define {{[^@]+}}@test_hoist_bool_attrs2
+; CHECK-SAME: (i1 [[C:%.*]], ptr [[P:%.*]], i64 [[X:%.*]]) {
+; CHECK-NEXT: [[R:%.*]] = call noundef ptr @foo(ptr nonnull [[P]], i64 noundef [[X]]) #[[ATTR2:[0-9]+]]
+; CHECK-NEXT: br i1 [[C]], label [[COMMON_RET:%.*]], label [[ELSE:%.*]]
+; CHECK: common.ret:
+; CHECK-NEXT: ret ptr [[R]]
+; CHECK: else:
+; CHECK-NEXT: call void @side.effect()
+; CHECK-NEXT: br label [[COMMON_RET]]
+;
+ br i1 %c, label %if, label %else
+if:
+ %r = call noundef ptr @foo(ptr readnone nonnull noundef %p, i64 noundef %x) cold mustprogress nocallback nofree nosync willreturn
+ ret ptr %r
+
+else:
+ %r2 = call noundef nonnull ptr @foo(ptr readonly nonnull %p, i64 noundef %x) mustprogress nocallback nofree willreturn
+ call void @side.effect()
+ ret ptr %r2
+}
+
+define ptr @test_hoist_bool_attrs3(i1 %c, ptr %p, i64 %x) {
+; CHECK-LABEL: define {{[^@]+}}@test_hoist_bool_attrs3
+; CHECK-SAME: (i1 [[C:%.*]], ptr [[P:%.*]], i64 [[X:%.*]]) {
+; CHECK-NEXT: [[R:%.*]] = call nonnull ptr @foo(ptr [[P]], i64 noundef [[X]]) #[[ATTR3:[0-9]+]]
+; CHECK-NEXT: br i1 [[C]], label [[COMMON_RET:%.*]], label [[ELSE:%.*]]
+; CHECK: common.ret:
+; CHECK-NEXT: ret ptr [[R]]
+; CHECK...
[truncated]
|
goldsteinn
changed the title
You can test this locally with the following command:git-clang-format --diff f7c3309b1361443cded697255995fade24838ef8 dbab2cf4825ee2c994581731b6e9e09726ba16ee --extensions h,cpp -- llvm/include/llvm/IR/Attributes.h llvm/include/llvm/IR/Instruction.h llvm/lib/IR/Attributes.cpp llvm/lib/IR/Instruction.cpp llvm/lib/Transforms/Utils/SimplifyCFG.cppView the diff from clang-format here.diff --git a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
index cd17dc454f..cfbc3155bd 100644
--- a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
+++ b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
@@ -1621,7 +1621,7 @@ static std::optional<AttributeList> tryIntersectAttrs(const CallBase *CB0,
if (!Attr.isValid())
return std::nullopt;
- // Only supporting enum attrs for now.
+ // Only supporting enum attrs for now.
if (!Attr.hasKindAsEnum())
return std::nullopt;
@@ -1699,7 +1699,7 @@ static std::optional<AttributeList> tryIntersectAttrs(const CallBase *CB0,
}
}
}
- return Intersected;
+ return Intersected;
};
// Intersect all attribute types (ret/fn/param).
@@ -2196,7 +2196,7 @@ static bool canSinkInstructions(
return false;
}
}
- // Check that we can intersect the attributes if we sink.
+ // Check that we can intersect the attributes if we sink.
for (const Instruction *I : Insts) {
if (I != I0 && !tryIntersectAttrs(CB, cast<CallBase>(I)))
return false;
|
dtcxzyw
reviewed
Sep 21, 2024
| @@ -1591,10 +1591,149 @@ static void hoistLockstepIdenticalDbgVariableRecords( | |||
| } | |||
| } | |||
|
|
|||
| // See if we can intersect the attributes for two callbases (used for sinking). | |||
There was a problem hiding this comment.
Suggested change
| // See if we can intersect the attributes for two callbases (used for sinking). | |
| /// See if we can intersect the attributes for two callbases (used for hoisting/sinking). |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Some (many) attributes can safely be dropped to enable sinking. For
example removing
nonnullon a return/param can't affect correctness.NB: Related to #91101