Skip to content

Commit 2477138

Browse files
nikicIanWood1
nikic
authored and
IanWood1
committed
[IR] Do not store Function inside BlockAddress (llvm#137958)
Currently BlockAddresses store both the Function and the BasicBlock they reference, and the BlockAddress is part of the use list of both the Function and BasicBlock. This is quite awkward, because this is not really a use of the function itself (and walks of function uses generally skip block addresses for that reason). This also has weird implications on function RAUW (as that will replace the function in block addresses in a way that generally doesn't make sense), and causes other peculiar issues, like the ability to have multiple block addresses for one block (with different functions). Instead, I believe it makes more sense to specify only the basic block and let the function be implied by the BB parent. This does mean that we may have block addresses without a function (if the BB is not inserted), but this should only happen during IR construction.
1 parent bbdf6e5 commit 2477138

File tree

19 files changed

+54
-125
lines changed

19 files changed

+54
-125
lines changed

‎clang/lib/CodeGen/CodeGenFunction.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2286,7 +2286,7 @@ llvm::BlockAddress *CodeGenFunction::GetAddrOfLabel(const LabelDecl *L) {
22862286

22872287
// Make sure the indirect branch includes all of the address-taken blocks.
22882288
IndirectBranch->addDestination(BB);
2289-
return llvm::BlockAddress::get(CurFn, BB);
2289+
return llvm::BlockAddress::get(CurFn->getType(), BB);
22902290
}
22912291

22922292
llvm::BasicBlock *CodeGenFunction::GetIndirectGotoBlock() {

‎llvm/include/llvm/IR/Constants.h

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -893,9 +893,9 @@ class ConstantTargetNone final : public ConstantData {
893893
class BlockAddress final : public Constant {
894894
friend class Constant;
895895

896-
constexpr static IntrusiveOperandsAllocMarker AllocMarker{2};
896+
constexpr static IntrusiveOperandsAllocMarker AllocMarker{1};
897897

898-
BlockAddress(Function *F, BasicBlock *BB);
898+
BlockAddress(Type *Ty, BasicBlock *BB);
899899

900900
void *operator new(size_t S) { return User::operator new(S, AllocMarker); }
901901

@@ -912,6 +912,11 @@ class BlockAddress final : public Constant {
912912
/// block must be embedded into a function.
913913
static BlockAddress *get(BasicBlock *BB);
914914

915+
/// Return a BlockAddress for the specified basic block, which may not be
916+
/// part of a function. The specified type must match the type of the function
917+
/// the block will be inserted into.
918+
static BlockAddress *get(Type *Ty, BasicBlock *BB);
919+
915920
/// Lookup an existing \c BlockAddress constant for the given BasicBlock.
916921
///
917922
/// \returns 0 if \c !BB->hasAddressTaken(), otherwise the \c BlockAddress.
@@ -920,8 +925,8 @@ class BlockAddress final : public Constant {
920925
/// Transparently provide more efficient getOperand methods.
921926
DECLARE_TRANSPARENT_OPERAND_ACCESSORS(Value);
922927

923-
Function *getFunction() const { return (Function *)Op<0>().get(); }
924-
BasicBlock *getBasicBlock() const { return (BasicBlock *)Op<1>().get(); }
928+
BasicBlock *getBasicBlock() const { return cast<BasicBlock>(Op<0>().get()); }
929+
Function *getFunction() const { return getBasicBlock()->getParent(); }
925930

926931
/// Methods for support type inquiry through isa, cast, and dyn_cast:
927932
static bool classof(const Value *V) {
@@ -931,7 +936,7 @@ class BlockAddress final : public Constant {
931936

932937
template <>
933938
struct OperandTraits<BlockAddress>
934-
: public FixedNumOperandTraits<BlockAddress, 2> {};
939+
: public FixedNumOperandTraits<BlockAddress, 1> {};
935940

936941
DEFINE_TRANSPARENT_OPERAND_ACCESSORS(BlockAddress, Value)
937942

‎llvm/lib/Bitcode/Reader/BitcodeReader.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1653,7 +1653,7 @@ Expected<Value *> BitcodeReader::materializeValue(unsigned StartValID,
16531653
FwdBBs[BBID] = BasicBlock::Create(Context);
16541654
BB = FwdBBs[BBID];
16551655
}
1656-
C = BlockAddress::get(Fn, BB);
1656+
C = BlockAddress::get(Fn->getType(), BB);
16571657
break;
16581658
}
16591659
case BitcodeConstant::ConstantStructOpcode: {

‎llvm/lib/CodeGen/AsmPrinter/WinCFGuard.cpp

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -49,10 +49,6 @@ static bool isPossibleIndirectCallTarget(const Function *F) {
4949
const Value *FnOrCast = Users.pop_back_val();
5050
for (const Use &U : FnOrCast->uses()) {
5151
const User *FnUser = U.getUser();
52-
if (isa<BlockAddress>(FnUser)) {
53-
// Block addresses are illegal to call.
54-
continue;
55-
}
5652
if (const auto *Call = dyn_cast<CallBase>(FnUser)) {
5753
if ((!Call->isCallee(&U) || U.get() != F) &&
5854
!Call->getFunction()->getName().ends_with("$exit_thunk")) {

‎llvm/lib/IR/Constants.cpp

Lines changed: 20 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -1891,77 +1891,61 @@ void PoisonValue::destroyConstantImpl() {
18911891
getContext().pImpl->PVConstants.erase(getType());
18921892
}
18931893

1894+
BlockAddress *BlockAddress::get(Type *Ty, BasicBlock *BB) {
1895+
BlockAddress *&BA = BB->getContext().pImpl->BlockAddresses[BB];
1896+
if (!BA)
1897+
BA = new BlockAddress(Ty, BB);
1898+
return BA;
1899+
}
1900+
18941901
BlockAddress *BlockAddress::get(BasicBlock *BB) {
18951902
assert(BB->getParent() && "Block must have a parent");
1896-
return get(BB->getParent(), BB);
1903+
return get(BB->getParent()->getType(), BB);
18971904
}
18981905

18991906
BlockAddress *BlockAddress::get(Function *F, BasicBlock *BB) {
1900-
BlockAddress *&BA =
1901-
F->getContext().pImpl->BlockAddresses[std::make_pair(F, BB)];
1902-
if (!BA)
1903-
BA = new BlockAddress(F, BB);
1904-
1905-
assert(BA->getFunction() == F && "Basic block moved between functions");
1906-
return BA;
1907+
assert(BB->getParent() == F && "Block not part of specified function");
1908+
return get(BB->getParent()->getType(), BB);
19071909
}
19081910

1909-
BlockAddress::BlockAddress(Function *F, BasicBlock *BB)
1910-
: Constant(PointerType::get(F->getContext(), F->getAddressSpace()),
1911-
Value::BlockAddressVal, AllocMarker) {
1912-
setOperand(0, F);
1913-
setOperand(1, BB);
1911+
BlockAddress::BlockAddress(Type *Ty, BasicBlock *BB)
1912+
: Constant(Ty, Value::BlockAddressVal, AllocMarker) {
1913+
setOperand(0, BB);
19141914
BB->AdjustBlockAddressRefCount(1);
19151915
}
19161916

19171917
BlockAddress *BlockAddress::lookup(const BasicBlock *BB) {
19181918
if (!BB->hasAddressTaken())
19191919
return nullptr;
19201920

1921-
const Function *F = BB->getParent();
1922-
assert(F && "Block must have a parent");
1923-
BlockAddress *BA =
1924-
F->getContext().pImpl->BlockAddresses.lookup(std::make_pair(F, BB));
1921+
BlockAddress *BA = BB->getContext().pImpl->BlockAddresses.lookup(BB);
19251922
assert(BA && "Refcount and block address map disagree!");
19261923
return BA;
19271924
}
19281925

19291926
/// Remove the constant from the constant table.
19301927
void BlockAddress::destroyConstantImpl() {
1931-
getFunction()->getType()->getContext().pImpl
1932-
->BlockAddresses.erase(std::make_pair(getFunction(), getBasicBlock()));
1928+
getType()->getContext().pImpl->BlockAddresses.erase(getBasicBlock());
19331929
getBasicBlock()->AdjustBlockAddressRefCount(-1);
19341930
}
19351931

19361932
Value *BlockAddress::handleOperandChangeImpl(Value *From, Value *To) {
1937-
// This could be replacing either the Basic Block or the Function. In either
1938-
// case, we have to remove the map entry.
1939-
Function *NewF = getFunction();
1940-
BasicBlock *NewBB = getBasicBlock();
1941-
1942-
if (From == NewF)
1943-
NewF = cast<Function>(To->stripPointerCasts());
1944-
else {
1945-
assert(From == NewBB && "From does not match any operand");
1946-
NewBB = cast<BasicBlock>(To);
1947-
}
1933+
assert(From == getBasicBlock());
1934+
BasicBlock *NewBB = cast<BasicBlock>(To);
19481935

19491936
// See if the 'new' entry already exists, if not, just update this in place
19501937
// and return early.
1951-
BlockAddress *&NewBA =
1952-
getContext().pImpl->BlockAddresses[std::make_pair(NewF, NewBB)];
1938+
BlockAddress *&NewBA = getContext().pImpl->BlockAddresses[NewBB];
19531939
if (NewBA)
19541940
return NewBA;
19551941

19561942
getBasicBlock()->AdjustBlockAddressRefCount(-1);
19571943

19581944
// Remove the old entry, this can't cause the map to rehash (just a
19591945
// tombstone will get added).
1960-
getContext().pImpl->BlockAddresses.erase(std::make_pair(getFunction(),
1961-
getBasicBlock()));
1946+
getContext().pImpl->BlockAddresses.erase(getBasicBlock());
19621947
NewBA = this;
1963-
setOperand(0, NewF);
1964-
setOperand(1, NewBB);
1948+
setOperand(0, NewBB);
19651949
getBasicBlock()->AdjustBlockAddressRefCount(1);
19661950

19671951
// If we just want to keep the existing value, then return null.

‎llvm/lib/IR/Function.cpp

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -966,9 +966,6 @@ bool Function::hasAddressTaken(const User **PutOffender,
966966
bool IgnoreCastedDirectCall) const {
967967
for (const Use &U : uses()) {
968968
const User *FU = U.getUser();
969-
if (isa<BlockAddress>(FU))
970-
continue;
971-
972969
if (IgnoreCallbackUses) {
973970
AbstractCallSite ACS(&U);
974971
if (ACS && ACS.isCallbackCall())
@@ -1033,12 +1030,7 @@ bool Function::isDefTriviallyDead() const {
10331030
!hasAvailableExternallyLinkage())
10341031
return false;
10351032

1036-
// Check if the function is used by anything other than a blockaddress.
1037-
for (const User *U : users())
1038-
if (!isa<BlockAddress>(U))
1039-
return false;
1040-
1041-
return true;
1033+
return use_empty();
10421034
}
10431035

10441036
/// callsFunctionThatReturnsTwice - Return true if the function has a call to

‎llvm/lib/IR/LLVMContextImpl.h

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1695,8 +1695,7 @@ class LLVMContextImpl {
16951695

16961696
StringMap<std::unique_ptr<ConstantDataSequential>> CDSConstants;
16971697

1698-
DenseMap<std::pair<const Function *, const BasicBlock *>, BlockAddress *>
1699-
BlockAddresses;
1698+
DenseMap<const BasicBlock *, BlockAddress *> BlockAddresses;
17001699

17011700
DenseMap<const GlobalValue *, DSOLocalEquivalent *> DSOLocalEquivalents;
17021701

‎llvm/lib/Transforms/IPO/Attributor.cpp

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1936,9 +1936,6 @@ bool Attributor::checkForAllCallSites(function_ref<bool(AbstractCallSite)> Pred,
19361936
LLVM_DEBUG(dbgs() << "[Attributor] Function " << Fn.getName()
19371937
<< " has non call site use " << *U.get() << " in "
19381938
<< *U.getUser() << "\n");
1939-
// BlockAddress users are allowed.
1940-
if (isa<BlockAddress>(U.getUser()))
1941-
continue;
19421939
return false;
19431940
}
19441941

@@ -3061,14 +3058,6 @@ ChangeStatus Attributor::rewriteFunctionSignatures(
30613058
// function empty.
30623059
NewFn->splice(NewFn->begin(), OldFn);
30633060

3064-
// Fixup block addresses to reference new function.
3065-
SmallVector<BlockAddress *, 8u> BlockAddresses;
3066-
for (User *U : OldFn->users())
3067-
if (auto *BA = dyn_cast<BlockAddress>(U))
3068-
BlockAddresses.push_back(BA);
3069-
for (auto *BA : BlockAddresses)
3070-
BA->replaceAllUsesWith(BlockAddress::get(NewFn, BA->getBasicBlock()));
3071-
30723061
// Set of all "call-like" instructions that invoke the old function mapped
30733062
// to their new replacements.
30743063
SmallVector<std::pair<CallBase *, CallBase *>, 8> CallSitePairs;

‎llvm/lib/Transforms/IPO/GlobalOpt.cpp

Lines changed: 3 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1678,11 +1678,8 @@ processGlobal(GlobalValue &GV,
16781678
/// Walk all of the direct calls of the specified function, changing them to
16791679
/// FastCC.
16801680
static void ChangeCalleesToFastCall(Function *F) {
1681-
for (User *U : F->users()) {
1682-
if (isa<BlockAddress>(U))
1683-
continue;
1681+
for (User *U : F->users())
16841682
cast<CallBase>(U)->setCallingConv(CallingConv::Fast);
1685-
}
16861683
}
16871684

16881685
static AttributeList StripAttr(LLVMContext &C, AttributeList Attrs,
@@ -1696,8 +1693,6 @@ static AttributeList StripAttr(LLVMContext &C, AttributeList Attrs,
16961693
static void RemoveAttribute(Function *F, Attribute::AttrKind A) {
16971694
F->setAttributes(StripAttr(F->getContext(), F->getAttributes(), A));
16981695
for (User *U : F->users()) {
1699-
if (isa<BlockAddress>(U))
1700-
continue;
17011696
CallBase *CB = cast<CallBase>(U);
17021697
CB->setAttributes(StripAttr(F->getContext(), CB->getAttributes(), A));
17031698
}
@@ -1722,8 +1717,6 @@ static bool hasChangeableCCImpl(Function *F) {
17221717
// Can't change CC of the function that either has musttail calls, or is a
17231718
// musttail callee itself
17241719
for (User *U : F->users()) {
1725-
if (isa<BlockAddress>(U))
1726-
continue;
17271720
CallInst* CI = dyn_cast<CallInst>(U);
17281721
if (!CI)
17291722
continue;
@@ -1772,9 +1765,6 @@ isValidCandidateForColdCC(Function &F,
17721765
return false;
17731766

17741767
for (User *U : F.users()) {
1775-
if (isa<BlockAddress>(U))
1776-
continue;
1777-
17781768
CallBase &CB = cast<CallBase>(*U);
17791769
Function *CallerFunc = CB.getParent()->getParent();
17801770
BlockFrequencyInfo &CallerBFI = GetBFI(*CallerFunc);
@@ -1787,11 +1777,8 @@ isValidCandidateForColdCC(Function &F,
17871777
}
17881778

17891779
static void changeCallSitesToColdCC(Function *F) {
1790-
for (User *U : F->users()) {
1791-
if (isa<BlockAddress>(U))
1792-
continue;
1780+
for (User *U : F->users())
17931781
cast<CallBase>(U)->setCallingConv(CallingConv::Cold);
1794-
}
17951782
}
17961783

17971784
// This function iterates over all the call instructions in the input Function
@@ -1832,12 +1819,7 @@ hasOnlyColdCalls(Function &F,
18321819

18331820
static bool hasMustTailCallers(Function *F) {
18341821
for (User *U : F->users()) {
1835-
CallBase *CB = dyn_cast<CallBase>(U);
1836-
if (!CB) {
1837-
assert(isa<BlockAddress>(U) &&
1838-
"Expected either CallBase or BlockAddress");
1839-
continue;
1840-
}
1822+
CallBase *CB = cast<CallBase>(U);
18411823
if (CB->isMustTailCall())
18421824
return true;
18431825
}

‎llvm/lib/Transforms/IPO/LowerTypeTests.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1920,9 +1920,9 @@ void LowerTypeTestsModule::replaceCfiUses(Function *Old, Value *New,
19201920
bool IsJumpTableCanonical) {
19211921
SmallSetVector<Constant *, 4> Constants;
19221922
for (Use &U : llvm::make_early_inc_range(Old->uses())) {
1923-
// Skip block addresses and no_cfi values, which refer to the function
1924-
// body instead of the jump table.
1925-
if (isa<BlockAddress, NoCFIValue>(U.getUser()))
1923+
// Skip no_cfi values, which refer to the function body instead of the jump
1924+
// table.
1925+
if (isa<NoCFIValue>(U.getUser()))
19261926
continue;
19271927

19281928
// Skip direct calls to externally defined or non-dso_local functions.

‎llvm/lib/Transforms/IPO/OpenMPOpt.cpp

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5734,10 +5734,7 @@ PreservedAnalyses OpenMPOptPass::run(Module &M, ModuleAnalysisManager &AM) {
57345734
auto IsCalled = [&](Function &F) {
57355735
if (Kernels.contains(&F))
57365736
return true;
5737-
for (const User *U : F.users())
5738-
if (!isa<BlockAddress>(U))
5739-
return true;
5740-
return false;
5737+
return !F.use_empty();
57415738
};
57425739

57435740
auto EmitRemark = [&](Function &F) {

‎llvm/lib/Transforms/IPO/PartialInlining.cpp

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -916,9 +916,6 @@ void PartialInlinerImpl::computeCallsiteToProfCountMap(
916916
};
917917

918918
for (User *User : Users) {
919-
// Don't bother with BlockAddress used by CallBr for asm goto.
920-
if (isa<BlockAddress>(User))
921-
continue;
922919
CallBase *CB = getSupportedCallBase(User);
923920
Function *Caller = CB->getCaller();
924921
if (CurrentCaller != Caller) {
@@ -1359,10 +1356,6 @@ bool PartialInlinerImpl::tryPartialInline(FunctionCloner &Cloner) {
13591356

13601357
bool AnyInline = false;
13611358
for (User *User : Users) {
1362-
// Don't bother with BlockAddress used by CallBr for asm goto.
1363-
if (isa<BlockAddress>(User))
1364-
continue;
1365-
13661359
CallBase *CB = getSupportedCallBase(User);
13671360

13681361
if (isLimitReached())

‎llvm/lib/Transforms/IPO/SCCP.cpp

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -318,11 +318,10 @@ static bool runIPSCCP(
318318
for (Use &U : F->uses()) {
319319
CallBase *CB = dyn_cast<CallBase>(U.getUser());
320320
if (!CB) {
321-
assert(isa<BlockAddress>(U.getUser()) ||
322-
(isa<Constant>(U.getUser()) &&
323-
all_of(U.getUser()->users(), [](const User *UserUser) {
324-
return cast<IntrinsicInst>(UserUser)->isAssumeLikeIntrinsic();
325-
})));
321+
assert(isa<Constant>(U.getUser()) &&
322+
all_of(U.getUser()->users(), [](const User *UserUser) {
323+
return cast<IntrinsicInst>(UserUser)->isAssumeLikeIntrinsic();
324+
}));
326325
continue;
327326
}
328327

‎llvm/test/tools/llvm-reduce/reduce-functions-blockaddress-wrong-function.ll

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,13 @@
33

44
; INTERESTING: @blockaddr.table.other
55

6-
; RESULT: @blockaddr.table.other = private unnamed_addr constant [2 x ptr] [ptr blockaddress(@bar, %L1), ptr blockaddress(@bar, %L2)]
6+
; RESULT: @blockaddr.table.other = private unnamed_addr constant [2 x ptr] [ptr inttoptr (i32 1 to ptr), ptr inttoptr (i32 1 to ptr)]
77

88

99
@blockaddr.table.other = private unnamed_addr constant [2 x ptr] [ptr blockaddress(@bar, %L1), ptr blockaddress(@bar, %L2)]
1010

1111

12-
; RESULT: define i32 @bar(
12+
; RESULT-NOT: define i32 @bar(
1313
define i32 @bar(i64 %arg0) {
1414
entry:
1515
%gep = getelementptr inbounds [2 x ptr], ptr @blockaddr.table.other, i64 0, i64 %arg0

0 commit comments

Comments
 (0)