Skip to content

Commit ed8c9c9

Browse files
rajatbajpairajatbajpai
committed
Addressed review comments
Simplify IRBuilder::CreateIntrinsic by delegating to the newer Intrinsic::getOrInsertDeclaration API, which now supports vararg intrinsics like experimental_gc_statepoint. Changes: - Remove duplicated intrinsic signature matching logic from IRBuilder::CreateIntrinsic and delegate to getOrInsertDeclaration - Extend Intrinsic::getOrInsertDeclaration to handle vararg intrinsics by recreating the FunctionType with isVarArg=true when needed - Add comprehensive unit tests covering non-overloaded, overloaded (scalar/vector), and vararg intrinsics for both direct API usage and IRBuilder
1 parent fe04f91 commit ed8c9c9

File tree

3 files changed

+177
-18
lines changed

3 files changed

+177
-18
lines changed

‎llvm/lib/IR/IRBuilder.cpp‎

Lines changed: 2 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -858,24 +858,12 @@ CallInst *IRBuilderBase::CreateIntrinsic(Type *RetTy, Intrinsic::ID ID,
858858
const Twine &Name) {
859859
Module *M = BB->getModule();
860860

861-
SmallVector<Intrinsic::IITDescriptor> Table;
862-
Intrinsic::getIntrinsicInfoTableEntries(ID, Table);
863-
ArrayRef<Intrinsic::IITDescriptor> TableRef(Table);
864-
865861
SmallVector<Type *> ArgTys;
866862
ArgTys.reserve(Args.size());
867863
for (auto &I : Args)
868864
ArgTys.push_back(I->getType());
869-
FunctionType *FTy = FunctionType::get(RetTy, ArgTys, false);
870-
SmallVector<Type *> OverloadTys;
871-
Intrinsic::MatchIntrinsicTypesResult Res =
872-
matchIntrinsicSignature(FTy, TableRef, OverloadTys);
873-
(void)Res;
874-
assert(Res == Intrinsic::MatchIntrinsicTypes_Match && TableRef.empty() &&
875-
"Wrong types for intrinsic!");
876-
// TODO: Handle varargs intrinsics.
877-
878-
Function *Fn = Intrinsic::getOrInsertDeclaration(M, ID, OverloadTys);
865+
866+
Function *Fn = Intrinsic::getOrInsertDeclaration(M, ID, RetTy, ArgTys);
879867
return createCallHelper(Fn, Args, Name, FMFSource);
880868
}
881869

‎llvm/lib/IR/Intrinsics.cpp‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -770,9 +770,15 @@ Function *Intrinsic::getOrInsertDeclaration(Module *M, ID IID, Type *RetTy,
770770
SmallVector<Type *, 4> OverloadTys;
771771
[[maybe_unused]] Intrinsic::MatchIntrinsicTypesResult Res =
772772
matchIntrinsicSignature(FTy, TableRef, OverloadTys);
773-
assert(Res == Intrinsic::MatchIntrinsicTypes_Match && TableRef.empty() &&
773+
assert(Res == Intrinsic::MatchIntrinsicTypes_Match &&
774774
"intrinsic signature mismatch");
775775

776+
// If intrinsic requires vararg, recreate the FunctionType accordingly.
777+
if (!matchIntrinsicVarArg(/*isVarArg=*/true, TableRef))
778+
FTy = FunctionType::get(RetTy, ArgTys, /*isVarArg=*/true);
779+
780+
assert(TableRef.empty() && "Unprocessed descriptors remain");
781+
776782
return getOrInsertIntrinsicDeclarationImpl(M, IID, OverloadTys, FTy);
777783
}
778784

‎llvm/unittests/IR/IntrinsicsTest.cpp‎

Lines changed: 168 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,14 +30,12 @@
3030
using namespace llvm;
3131

3232
namespace {
33-
3433
class IntrinsicsTest : public ::testing::Test {
34+
protected:
3535
LLVMContext Context;
3636
std::unique_ptr<Module> M;
3737
BasicBlock *BB = nullptr;
3838

39-
void TearDown() override { M.reset(); }
40-
4139
void SetUp() override {
4240
M = std::make_unique<Module>("Test", Context);
4341
auto F = M->getOrInsertFunction(
@@ -46,6 +44,8 @@ class IntrinsicsTest : public ::testing::Test {
4644
EXPECT_NE(BB, nullptr);
4745
}
4846

47+
void TearDown() override { M.reset(); }
48+
4949
public:
5050
Instruction *makeIntrinsic(Intrinsic::ID ID) const {
5151
IRBuilder<> Builder(BB);
@@ -197,4 +197,169 @@ TEST(IntrinsicAttributes, TestGetFnAttributesBug) {
197197
AttributeSet AS = getFnAttributes(Context, experimental_guard);
198198
EXPECT_FALSE(AS.hasAttributes());
199199
}
200+
201+
// Test non-overloaded intrinsic
202+
TEST_F(IntrinsicsTest, NonOverloadedIntrinsic) {
203+
Type *RetTy = Type::getVoidTy(Context);
204+
SmallVector<Type *, 1> ArgTys;
205+
ArgTys.push_back(Type::getInt1Ty(Context));
206+
207+
Function *F = Intrinsic::getOrInsertDeclaration(M.get(), Intrinsic::assume,
208+
RetTy, ArgTys);
209+
210+
ASSERT_NE(F, nullptr);
211+
EXPECT_EQ(F->getIntrinsicID(), Intrinsic::assume);
212+
EXPECT_EQ(F->getReturnType(), RetTy);
213+
EXPECT_EQ(F->arg_size(), 1u);
214+
EXPECT_FALSE(F->isVarArg());
215+
EXPECT_EQ(F->getName(), "llvm.assume");
216+
}
217+
218+
// Test overloaded intrinsic with automatic type resolution (scalar)
219+
TEST_F(IntrinsicsTest, OverloadedIntrinsicScalar) {
220+
Type *RetTy = Type::getInt32Ty(Context);
221+
SmallVector<Type *, 2> ArgTys;
222+
ArgTys.push_back(Type::getInt32Ty(Context));
223+
ArgTys.push_back(Type::getInt32Ty(Context));
224+
225+
Function *F = Intrinsic::getOrInsertDeclaration(M.get(), Intrinsic::umax,
226+
RetTy, ArgTys);
227+
228+
ASSERT_NE(F, nullptr);
229+
EXPECT_EQ(F->getIntrinsicID(), Intrinsic::umax);
230+
EXPECT_EQ(F->getReturnType(), RetTy);
231+
EXPECT_EQ(F->arg_size(), 2u);
232+
EXPECT_FALSE(F->isVarArg());
233+
EXPECT_EQ(F->getName(), "llvm.umax.i32");
234+
}
235+
236+
// Test overloaded intrinsic with automatic type resolution (vector)
237+
TEST_F(IntrinsicsTest, OverloadedIntrinsicVector) {
238+
Type *RetTy = FixedVectorType::get(Type::getInt32Ty(Context), 4);
239+
SmallVector<Type *, 2> ArgTys;
240+
ArgTys.push_back(RetTy);
241+
ArgTys.push_back(RetTy);
242+
243+
Function *F = Intrinsic::getOrInsertDeclaration(M.get(), Intrinsic::umax,
244+
RetTy, ArgTys);
245+
246+
ASSERT_NE(F, nullptr);
247+
EXPECT_EQ(F->getIntrinsicID(), Intrinsic::umax);
248+
EXPECT_EQ(F->getReturnType(), RetTy);
249+
EXPECT_EQ(F->arg_size(), 2u);
250+
EXPECT_FALSE(F->isVarArg());
251+
EXPECT_EQ(F->getName(), "llvm.umax.v4i32");
252+
}
253+
254+
// Test vararg intrinsic: experimental_gc_statepoint
255+
TEST_F(IntrinsicsTest, VarArgIntrinsicStatepoint) {
256+
Type *RetTy = Type::getTokenTy(Context);
257+
SmallVector<Type *, 5> ArgTys;
258+
ArgTys.push_back(Type::getInt64Ty(Context)); // ID
259+
ArgTys.push_back(Type::getInt32Ty(Context)); // NumPatchBytes
260+
ArgTys.push_back(PointerType::get(Context, 0)); // Target
261+
ArgTys.push_back(Type::getInt32Ty(Context)); // NumCallArgs
262+
ArgTys.push_back(Type::getInt32Ty(Context)); // Flags
263+
264+
Function *F = Intrinsic::getOrInsertDeclaration(
265+
M.get(), Intrinsic::experimental_gc_statepoint, RetTy, ArgTys);
266+
267+
ASSERT_NE(F, nullptr);
268+
EXPECT_EQ(F->getIntrinsicID(), Intrinsic::experimental_gc_statepoint);
269+
EXPECT_EQ(F->getReturnType(), RetTy);
270+
EXPECT_EQ(F->arg_size(), 5u);
271+
EXPECT_TRUE(F->isVarArg()) << "experimental_gc_statepoint must be vararg";
272+
EXPECT_EQ(F->getName(), "llvm.experimental.gc.statepoint.p0");
273+
}
274+
275+
// Test different overloads create different declarations
276+
TEST_F(IntrinsicsTest, DifferentOverloads) {
277+
// i32 version
278+
Type *RetTy32 = Type::getInt32Ty(Context);
279+
SmallVector<Type *, 2> ArgTys32;
280+
ArgTys32.push_back(Type::getInt32Ty(Context));
281+
ArgTys32.push_back(Type::getInt32Ty(Context));
282+
283+
Function *F32 = Intrinsic::getOrInsertDeclaration(M.get(), Intrinsic::umax,
284+
RetTy32, ArgTys32);
285+
286+
// i64 version
287+
Type *RetTy64 = Type::getInt64Ty(Context);
288+
SmallVector<Type *, 2> ArgTys64;
289+
ArgTys64.push_back(Type::getInt64Ty(Context));
290+
ArgTys64.push_back(Type::getInt64Ty(Context));
291+
292+
Function *F64 = Intrinsic::getOrInsertDeclaration(M.get(), Intrinsic::umax,
293+
RetTy64, ArgTys64);
294+
295+
EXPECT_NE(F32, F64) << "Different overloads should be different functions";
296+
EXPECT_EQ(F32->getName(), "llvm.umax.i32");
297+
EXPECT_EQ(F64->getName(), "llvm.umax.i64");
298+
}
299+
300+
// Test IRBuilder::CreateIntrinsic with overloaded scalar type
301+
TEST_F(IntrinsicsTest, IRBuilderCreateIntrinsicScalar) {
302+
IRBuilder<> Builder(BB);
303+
304+
Type *RetTy = Type::getInt32Ty(Context);
305+
SmallVector<Value *, 2> Args;
306+
Args.push_back(ConstantInt::get(Type::getInt32Ty(Context), 10));
307+
Args.push_back(ConstantInt::get(Type::getInt32Ty(Context), 20));
308+
309+
CallInst *CI = Builder.CreateIntrinsic(RetTy, Intrinsic::umax, Args);
310+
311+
ASSERT_NE(CI, nullptr);
312+
EXPECT_EQ(CI->getIntrinsicID(), Intrinsic::umax);
313+
EXPECT_EQ(CI->getType(), RetTy);
314+
EXPECT_EQ(CI->arg_size(), 2u);
315+
EXPECT_FALSE(CI->getCalledFunction()->isVarArg());
316+
}
317+
318+
// Test IRBuilder::CreateIntrinsic with vector intrinsic
319+
TEST_F(IntrinsicsTest, IRBuilderCreateIntrinsicVector) {
320+
IRBuilder<> Builder(BB);
321+
322+
Type *RetTy = FixedVectorType::get(Type::getInt32Ty(Context), 4);
323+
SmallVector<Value *, 2> Args;
324+
Args.push_back(Constant::getNullValue(RetTy));
325+
Args.push_back(Constant::getNullValue(RetTy));
326+
327+
CallInst *CI = Builder.CreateIntrinsic(RetTy, Intrinsic::umax, Args);
328+
329+
ASSERT_NE(CI, nullptr);
330+
EXPECT_EQ(CI->getIntrinsicID(), Intrinsic::umax);
331+
EXPECT_EQ(CI->getType(), RetTy);
332+
EXPECT_EQ(CI->arg_size(), 2u);
333+
EXPECT_FALSE(CI->getCalledFunction()->isVarArg());
334+
}
335+
336+
// Test IRBuilder::CreateIntrinsic with vararg intrinsic
337+
TEST_F(IntrinsicsTest, IRBuilderCreateIntrinsicVarArg) {
338+
IRBuilder<> Builder(BB);
339+
340+
// Create a dummy function to call through statepoint
341+
FunctionType *DummyFnTy = FunctionType::get(Type::getVoidTy(Context), false);
342+
Function *DummyFn = Function::Create(DummyFnTy, GlobalValue::ExternalLinkage,
343+
"dummy", M.get());
344+
345+
Type *RetTy = Type::getTokenTy(Context);
346+
SmallVector<Value *, 5> Args;
347+
Args.push_back(ConstantInt::get(Type::getInt64Ty(Context), 0)); // ID
348+
Args.push_back(
349+
ConstantInt::get(Type::getInt32Ty(Context), 0)); // NumPatchBytes
350+
Args.push_back(DummyFn); // Target
351+
Args.push_back(ConstantInt::get(Type::getInt32Ty(Context), 0)); // NumCallArgs
352+
Args.push_back(ConstantInt::get(Type::getInt32Ty(Context), 0)); // Flags
353+
354+
CallInst *CI = Builder.CreateIntrinsic(
355+
RetTy, Intrinsic::experimental_gc_statepoint, Args);
356+
357+
ASSERT_NE(CI, nullptr);
358+
EXPECT_EQ(CI->getIntrinsicID(), Intrinsic::experimental_gc_statepoint);
359+
EXPECT_EQ(CI->getType(), RetTy);
360+
EXPECT_EQ(CI->arg_size(), 5u);
361+
EXPECT_TRUE(CI->getCalledFunction()->isVarArg())
362+
<< "experimental_gc_statepoint must be vararg";
363+
}
364+
200365
} // end namespace

0 commit comments

Comments
 (0)