Skip to content

Commit 66ad147

Browse files
vporpovporpo
committed
[SandboxIR] Remove tight-coupling with LLVM's SwitchInst::CaseHandle
SandboxIR's SwitchInst CaseHandle was relying on LLVM IR's SwitchInst::CaseHandleImpl template, which may call private functions of SandboxIR's SwitchInst. This creates a dependency cycle which is against the design principles of Sandbox IR. The issue was exposed by: #166842 Thanks to @aengelke for raising the issue.
1 parent 917d815 commit 66ad147

File tree

2 files changed

+112
-21
lines changed

2 files changed

+112
-21
lines changed

‎llvm/include/llvm/SandboxIR/Instruction.h‎

Lines changed: 85 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1884,45 +1884,109 @@ class SwitchInst : public SingleLLVMInstructionImpl<llvm::SwitchInst> {
18841884
return cast<llvm::SwitchInst>(Val)->getNumCases();
18851885
}
18861886

1887+
template <typename LLVMCaseItT, typename BlockT, typename ConstT>
1888+
class CaseItImpl;
1889+
1890+
template <typename LLVMCaseItT, typename BlockT, typename ConstT>
1891+
class CaseHandleImpl {
1892+
Context &Ctx;
1893+
LLVMCaseItT LLVMCaseIt;
1894+
template <typename T1, typename T2, typename T3> friend class CaseItImpl;
1895+
1896+
public:
1897+
CaseHandleImpl(Context &Ctx, LLVMCaseItT LLVMCaseIt)
1898+
: Ctx(Ctx), LLVMCaseIt(LLVMCaseIt) {}
1899+
ConstT *getCaseValue() const;
1900+
BlockT *getCaseSuccessor() const;
1901+
unsigned getCaseIndex() const {
1902+
const auto &LLVMCaseHandle = *LLVMCaseIt;
1903+
return LLVMCaseHandle.getCaseIndex();
1904+
}
1905+
unsigned getSuccessorIndex() const {
1906+
const auto &LLVMCaseHandle = *LLVMCaseIt;
1907+
return LLVMCaseHandle.getSuccessorIndex();
1908+
}
1909+
};
1910+
1911+
template <typename LLVMCaseItT, typename BlockT, typename ConstT>
1912+
class CaseItImpl : public iterator_facade_base<
1913+
CaseItImpl<LLVMCaseItT, BlockT, ConstT>,
1914+
std::random_access_iterator_tag,
1915+
const CaseHandleImpl<LLVMCaseItT, BlockT, ConstT>> {
1916+
CaseHandleImpl<LLVMCaseItT, BlockT, ConstT> CH;
1917+
1918+
public:
1919+
CaseItImpl(Context &Ctx, LLVMCaseItT It) : CH(Ctx, It) {}
1920+
CaseItImpl(SwitchInst *SI, ptrdiff_t CaseNum)
1921+
: CH(SI->getContext(), llvm::SwitchInst::CaseIt(
1922+
cast<llvm::SwitchInst>(SI->Val), CaseNum)) {}
1923+
CaseItImpl &operator+=(ptrdiff_t N) {
1924+
CH.LLVMCaseIt.operator+=(N);
1925+
return *this;
1926+
}
1927+
CaseItImpl &operator-=(ptrdiff_t N) {
1928+
CH.LLVMCaseIt.operator-=(N);
1929+
return *this;
1930+
}
1931+
ptrdiff_t operator-(const CaseItImpl &Other) const {
1932+
return CH.LLVMCaseIt - Other.CH.LLVMCaseIt;
1933+
}
1934+
bool operator==(const CaseItImpl &Other) const {
1935+
return CH.LLVMCaseIt == Other.CH.LLVMCaseIt;
1936+
}
1937+
bool operator<(const CaseItImpl &Other) const {
1938+
return CH.LLVMCaseIt < Other.CH.LLVMCaseIt;
1939+
}
1940+
const CaseHandleImpl<LLVMCaseItT, BlockT, ConstT> &operator*() const {
1941+
return CH;
1942+
}
1943+
};
1944+
18871945
using CaseHandle =
1888-
llvm::SwitchInst::CaseHandleImpl<SwitchInst, ConstantInt, BasicBlock>;
1889-
using ConstCaseHandle =
1890-
llvm::SwitchInst::CaseHandleImpl<const SwitchInst, const ConstantInt,
1891-
const BasicBlock>;
1892-
using CaseIt = llvm::SwitchInst::CaseIteratorImpl<CaseHandle>;
1893-
using ConstCaseIt = llvm::SwitchInst::CaseIteratorImpl<ConstCaseHandle>;
1946+
CaseHandleImpl<llvm::SwitchInst::CaseIt, BasicBlock, ConstantInt>;
1947+
using CaseIt = CaseItImpl<llvm::SwitchInst::CaseIt, BasicBlock, ConstantInt>;
1948+
1949+
using ConstCaseHandle = CaseHandleImpl<llvm::SwitchInst::ConstCaseIt,
1950+
const BasicBlock, const ConstantInt>;
1951+
using ConstCaseIt = CaseItImpl<llvm::SwitchInst::ConstCaseIt,
1952+
const BasicBlock, const ConstantInt>;
18941953

18951954
/// Returns a read/write iterator that points to the first case in the
18961955
/// SwitchInst.
1897-
CaseIt case_begin() { return CaseIt(this, 0); }
1898-
ConstCaseIt case_begin() const { return ConstCaseIt(this, 0); }
1956+
CaseIt case_begin() {
1957+
return CaseIt(Ctx, cast<llvm::SwitchInst>(Val)->case_begin());
1958+
}
1959+
ConstCaseIt case_begin() const {
1960+
return ConstCaseIt(Ctx, cast<llvm::SwitchInst>(Val)->case_begin());
1961+
}
18991962
/// Returns a read/write iterator that points one past the last in the
19001963
/// SwitchInst.
1901-
CaseIt case_end() { return CaseIt(this, getNumCases()); }
1902-
ConstCaseIt case_end() const { return ConstCaseIt(this, getNumCases()); }
1964+
CaseIt case_end() {
1965+
return CaseIt(Ctx, cast<llvm::SwitchInst>(Val)->case_end());
1966+
}
1967+
ConstCaseIt case_end() const {
1968+
return ConstCaseIt(Ctx, cast<llvm::SwitchInst>(Val)->case_end());
1969+
}
19031970
/// Iteration adapter for range-for loops.
19041971
iterator_range<CaseIt> cases() {
19051972
return make_range(case_begin(), case_end());
19061973
}
19071974
iterator_range<ConstCaseIt> cases() const {
19081975
return make_range(case_begin(), case_end());
19091976
}
1910-
CaseIt case_default() { return CaseIt(this, DefaultPseudoIndex); }
1977+
CaseIt case_default() {
1978+
return CaseIt(Ctx, cast<llvm::SwitchInst>(Val)->case_default());
1979+
}
19111980
ConstCaseIt case_default() const {
1912-
return ConstCaseIt(this, DefaultPseudoIndex);
1981+
return ConstCaseIt(Ctx, cast<llvm::SwitchInst>(Val)->case_default());
19131982
}
19141983
CaseIt findCaseValue(const ConstantInt *C) {
1915-
return CaseIt(
1916-
this,
1917-
const_cast<const SwitchInst *>(this)->findCaseValue(C)->getCaseIndex());
1984+
const llvm::ConstantInt *LLVMC = cast<llvm::ConstantInt>(C->Val);
1985+
return CaseIt(Ctx, cast<llvm::SwitchInst>(Val)->findCaseValue(LLVMC));
19181986
}
19191987
ConstCaseIt findCaseValue(const ConstantInt *C) const {
1920-
ConstCaseIt I = llvm::find_if(cases(), [C](const ConstCaseHandle &Case) {
1921-
return Case.getCaseValue() == C;
1922-
});
1923-
if (I != case_end())
1924-
return I;
1925-
return case_default();
1988+
const llvm::ConstantInt *LLVMC = cast<llvm::ConstantInt>(C->Val);
1989+
return ConstCaseIt(Ctx, cast<llvm::SwitchInst>(Val)->findCaseValue(LLVMC));
19261990
}
19271991
LLVM_ABI ConstantInt *findCaseDest(BasicBlock *BB);
19281992

‎llvm/lib/SandboxIR/Instruction.cpp‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1125,6 +1125,33 @@ void SwitchInst::setDefaultDest(BasicBlock *DefaultCase) {
11251125
cast<llvm::SwitchInst>(Val)->setDefaultDest(
11261126
cast<llvm::BasicBlock>(DefaultCase->Val));
11271127
}
1128+
1129+
template <typename LLVMCaseItT, typename BlockT, typename ConstT>
1130+
ConstT *
1131+
SwitchInst::CaseHandleImpl<LLVMCaseItT, BlockT, ConstT>::getCaseValue() const {
1132+
const auto &LLVMCaseHandle = *LLVMCaseIt;
1133+
auto *LLVMC = Ctx.getValue(LLVMCaseHandle.getCaseValue());
1134+
return cast<ConstT>(LLVMC);
1135+
}
1136+
1137+
template <typename LLVMCaseItT, typename BlockT, typename ConstT>
1138+
BlockT *
1139+
SwitchInst::CaseHandleImpl<LLVMCaseItT, BlockT, ConstT>::getCaseSuccessor()
1140+
const {
1141+
const auto &LLVMCaseHandle = *LLVMCaseIt;
1142+
auto *LLVMBB = LLVMCaseHandle.getCaseSuccessor();
1143+
return cast<BlockT>(Ctx.getValue(LLVMBB));
1144+
}
1145+
1146+
template class SwitchInst::CaseHandleImpl<llvm::SwitchInst::CaseIt, BasicBlock,
1147+
ConstantInt>;
1148+
template class SwitchInst::CaseItImpl<llvm::SwitchInst::CaseIt, BasicBlock,
1149+
ConstantInt>;
1150+
template class SwitchInst::CaseHandleImpl<llvm::SwitchInst::ConstCaseIt,
1151+
const BasicBlock, const ConstantInt>;
1152+
template class SwitchInst::CaseItImpl<llvm::SwitchInst::ConstCaseIt,
1153+
const BasicBlock, const ConstantInt>;
1154+
11281155
ConstantInt *SwitchInst::findCaseDest(BasicBlock *BB) {
11291156
auto *LLVMC = cast<llvm::SwitchInst>(Val)->findCaseDest(
11301157
cast<llvm::BasicBlock>(BB->Val));

0 commit comments

Comments
 (0)