https://github.com/andykaylor created https://github.com/llvm/llvm-project/pull/184654
We were attempting to capture a `count` argument for __builtin_va_start but the builtin doesn't actually have that as a defined argument. This change removes the errant handling. >From 38d7d3a66f7cd8d98d82d70c9ce3174ec7a740c5 Mon Sep 17 00:00:00 2001 From: Andy Kaylor <[email protected]> Date: Wed, 4 Mar 2026 09:14:12 -0800 Subject: [PATCH] [CIR] Fix __builtin_va_start handling We were attempting to capture a `count` argument for __builtin_va_start but the builtin doesn't actually have that as a defined argument. This change removes the errant handling. --- clang/include/clang/CIR/Dialect/IR/CIROps.td | 18 ++--- clang/lib/CIR/CodeGen/CIRGenBuiltin.cpp | 7 +- clang/lib/CIR/CodeGen/CIRGenFunction.h | 4 +- clang/test/CIR/CodeGen/var-arg-aggregate.c | 5 +- clang/test/CIR/CodeGen/var_arg.c | 84 ++++++++++++++++++-- 5 files changed, 87 insertions(+), 31 deletions(-) diff --git a/clang/include/clang/CIR/Dialect/IR/CIROps.td b/clang/include/clang/CIR/Dialect/IR/CIROps.td index 064be6f54def2..532fa420da588 100644 --- a/clang/include/clang/CIR/Dialect/IR/CIROps.td +++ b/clang/include/clang/CIR/Dialect/IR/CIROps.td @@ -6009,11 +6009,9 @@ def CIR_VAStartOp : CIR_Op<"va_start"> { initializing a variable argument list at the given va_list storage location. - The first operand must be a pointer to the target's `va_list` - representation. This operation has no results and produces its effect by - mutating the storage referenced by the pointer operand. The second operand - must be an integer value that contains the expected number of arguments in - that list. + The operand must be a pointer to the target's `va_list` representation. + This operation has no results and produces its effect by mutating the + storage referenced by the pointer operand. Each `cir.va_start` must be paired with a corresponding `cir.va_end` on the same logical `va_list` object along all control-flow paths. After @@ -6030,17 +6028,13 @@ def CIR_VAStartOp : CIR_Op<"va_start"> { %p = cir.cast array_to_ptrdecay %args : !cir.ptr<!cir.array<!rec___va_list_tag x 1>>) -> !cir.ptr<!rec___va_list_tag> - %count = cir.load %0 : !cir.ptr<!s32i>, !s32i - cir.va_start %p %count : !cir.ptr<!rec___va_list_tag>, !s32i + cir.va_start %p : !cir.ptr<!rec___va_list_tag> ``` }]; - let arguments = (ins - CIR_PointerType:$arg_list, - CIR_AnyFundamentalIntType:$count - ); + let arguments = (ins CIR_PointerType:$arg_list); let assemblyFormat = [{ - $arg_list $count attr-dict `:` type(operands) + $arg_list attr-dict `:` type(operands) }]; } diff --git a/clang/lib/CIR/CodeGen/CIRGenBuiltin.cpp b/clang/lib/CIR/CodeGen/CIRGenBuiltin.cpp index 86d34be0a311c..94bd0ccdc9fde 100644 --- a/clang/lib/CIR/CodeGen/CIRGenBuiltin.cpp +++ b/clang/lib/CIR/CodeGen/CIRGenBuiltin.cpp @@ -966,8 +966,7 @@ RValue CIRGenFunction::emitBuiltinExpr(const GlobalDecl &gd, unsigned builtinID, mlir::Value vaList = builtinID == Builtin::BI__va_start ? emitScalarExpr(e->getArg(0)) : emitVAListRef(e->getArg(0)).getPointer(); - mlir::Value count = emitScalarExpr(e->getArg(1)); - emitVAStart(vaList, count); + emitVAStart(vaList); return {}; } @@ -2391,10 +2390,10 @@ mlir::Value CIRGenFunction::emitCheckedArgForAssume(const Expr *e) { return {}; } -void CIRGenFunction::emitVAStart(mlir::Value vaList, mlir::Value count) { +void CIRGenFunction::emitVAStart(mlir::Value vaList) { // LLVM codegen casts to *i8, no real gain on doing this for CIRGen this // early, defer to LLVM lowering. - cir::VAStartOp::create(builder, vaList.getLoc(), vaList, count); + cir::VAStartOp::create(builder, vaList.getLoc(), vaList); } void CIRGenFunction::emitVAEnd(mlir::Value vaList) { diff --git a/clang/lib/CIR/CodeGen/CIRGenFunction.h b/clang/lib/CIR/CodeGen/CIRGenFunction.h index 2af3f35e355b1..56bdcfd7f8906 100644 --- a/clang/lib/CIR/CodeGen/CIRGenFunction.h +++ b/clang/lib/CIR/CodeGen/CIRGenFunction.h @@ -2014,9 +2014,7 @@ class CIRGenFunction : public CIRGenTypeCache { /// /// \param vaList A reference to the \c va_list as emitted by either /// \c emitVAListRef or \c emitMSVAListRef. - /// - /// \param count The number of arguments in \c vaList - void emitVAStart(mlir::Value vaList, mlir::Value count); + void emitVAStart(mlir::Value vaList); /// Emits the end of a CIR variable-argument operation (`cir.va_start`) /// diff --git a/clang/test/CIR/CodeGen/var-arg-aggregate.c b/clang/test/CIR/CodeGen/var-arg-aggregate.c index 6cb623821f8b3..78958576d02b6 100644 --- a/clang/test/CIR/CodeGen/var-arg-aggregate.c +++ b/clang/test/CIR/CodeGen/var-arg-aggregate.c @@ -20,14 +20,11 @@ struct Bar varargs_aggregate(int count, ...) { } // CIR-LABEL: cir.func {{.*}} @varargs_aggregate( -// CIR: %[[COUNT_ADDR:.+]] = cir.alloca !s32i, !cir.ptr<!s32i>, ["count", init] // CIR: %[[RET_ADDR:.+]] = cir.alloca !rec_Bar, !cir.ptr<!rec_Bar>, ["__retval", init] // CIR: %[[VAAREA:.+]] = cir.alloca !cir.array<!rec___va_list_tag x 1>, !cir.ptr<!cir.array<!rec___va_list_tag x 1>>, ["args"] // CIR: %[[TMP_ADDR:.+]] = cir.alloca !rec_Bar, !cir.ptr<!rec_Bar>, ["vaarg.tmp"] -// CIR: cir.store %arg0, %[[COUNT_ADDR]] : !s32i, !cir.ptr<!s32i> // CIR: %[[VA_PTR0:.+]] = cir.cast array_to_ptrdecay %[[VAAREA]] : !cir.ptr<!cir.array<!rec___va_list_tag x 1>> -> !cir.ptr<!rec___va_list_tag> -// CIR: %[[COUNT_VAL:.+]] = cir.load{{.*}} %[[COUNT_ADDR]] : !cir.ptr<!s32i>, !s32i -// CIR: cir.va_start %[[VA_PTR0]] %[[COUNT_VAL]] : !cir.ptr<!rec___va_list_tag>, !s32i +// CIR: cir.va_start %[[VA_PTR0]] : !cir.ptr<!rec___va_list_tag> // CIR: %[[VA_PTR1:.+]] = cir.cast array_to_ptrdecay %[[VAAREA]] : !cir.ptr<!cir.array<!rec___va_list_tag x 1>> -> !cir.ptr<!rec___va_list_tag> // CIR: %[[VA_ARG:.+]] = cir.va_arg %[[VA_PTR1]] : (!cir.ptr<!rec___va_list_tag>) -> !rec_Bar // CIR: cir.store{{.*}} %[[VA_ARG]], %[[TMP_ADDR]] : !rec_Bar, !cir.ptr<!rec_Bar> diff --git a/clang/test/CIR/CodeGen/var_arg.c b/clang/test/CIR/CodeGen/var_arg.c index aab909eb67672..2fad7cd890b7d 100644 --- a/clang/test/CIR/CodeGen/var_arg.c +++ b/clang/test/CIR/CodeGen/var_arg.c @@ -18,14 +18,11 @@ int varargs(int count, ...) { } // CIR-LABEL: cir.func {{.*}} @varargs( -// CIR: %[[COUNT_ADDR:.+]] = cir.alloca !s32i, !cir.ptr<!s32i>, ["count", init] // CIR: %[[RET_ADDR:.+]] = cir.alloca !s32i, !cir.ptr<!s32i>, ["__retval"] // CIR: %[[VAAREA:.+]] = cir.alloca !cir.array<!rec___va_list_tag x 1>, !cir.ptr<!cir.array<!rec___va_list_tag x 1>>, ["args"] // CIR: %[[RES_ADDR:.+]] = cir.alloca !s32i, !cir.ptr<!s32i>, ["res", init] -// CIR: cir.store %arg0, %[[COUNT_ADDR]] : !s32i, !cir.ptr<!s32i> // CIR: %[[VA_PTR0:.+]] = cir.cast array_to_ptrdecay %[[VAAREA]] : !cir.ptr<!cir.array<!rec___va_list_tag x 1>> -> !cir.ptr<!rec___va_list_tag> -// CIR: %[[COUNT_VAL:.+]] = cir.load{{.*}} %[[COUNT_ADDR]] : !cir.ptr<!s32i>, !s32i -// CIR: cir.va_start %[[VA_PTR0]] %[[COUNT_VAL]] : !cir.ptr<!rec___va_list_tag>, !s32i +// CIR: cir.va_start %[[VA_PTR0]] : !cir.ptr<!rec___va_list_tag> // CIR: %[[VA_PTR1:.+]] = cir.cast array_to_ptrdecay %[[VAAREA]] : !cir.ptr<!cir.array<!rec___va_list_tag x 1>> -> !cir.ptr<!rec___va_list_tag> // CIR: %[[VA_ARG:.+]] = cir.va_arg %[[VA_PTR1]] : (!cir.ptr<!rec___va_list_tag>) -> !s32i // CIR: cir.store{{.*}} %[[VA_ARG]], %[[RES_ADDR]] : !s32i, !cir.ptr<!s32i> @@ -94,14 +91,11 @@ int stdarg_start(int count, ...) { } // CIR-LABEL: cir.func {{.*}} @stdarg_start( -// CIR: %[[COUNT_ADDR:.+]] = cir.alloca !s32i, !cir.ptr<!s32i>, ["count", init] // CIR: %[[RET_ADDR:.+]] = cir.alloca !s32i, !cir.ptr<!s32i>, ["__retval"] // CIR: %[[VAAREA:.+]] = cir.alloca !cir.array<!rec___va_list_tag x 1>, !cir.ptr<!cir.array<!rec___va_list_tag x 1>>, ["args"] // CIR: %[[RES_ADDR:.+]] = cir.alloca !s32i, !cir.ptr<!s32i>, ["res", init] -// CIR: cir.store %arg0, %[[COUNT_ADDR]] : !s32i, !cir.ptr<!s32i> // CIR: %[[VA_PTR0:.+]] = cir.cast array_to_ptrdecay %[[VAAREA]] : !cir.ptr<!cir.array<!rec___va_list_tag x 1>> -> !cir.ptr<!rec___va_list_tag> -// CIR: %[[C12345:.+]] = cir.const #cir.int<12345> : !s32i -// CIR: cir.va_start %[[VA_PTR0]] %[[C12345]] : !cir.ptr<!rec___va_list_tag>, !s32i +// CIR: cir.va_start %[[VA_PTR0]] : !cir.ptr<!rec___va_list_tag> // CIR: %[[VA_PTR1:.+]] = cir.cast array_to_ptrdecay %[[VAAREA]] : !cir.ptr<!cir.array<!rec___va_list_tag x 1>> -> !cir.ptr<!rec___va_list_tag> // CIR: %[[VA_ARG:.+]] = cir.va_arg %[[VA_PTR1]] : (!cir.ptr<!rec___va_list_tag>) -> !s32i // CIR: cir.store{{.*}} %[[VA_ARG]], %[[RES_ADDR]] : !s32i, !cir.ptr<!s32i> @@ -184,3 +178,77 @@ void stdarg_copy() { // OGCG: %{{.*}} = getelementptr inbounds [1 x %struct.__va_list_tag], ptr %{{.*}} // OGCG: %{{.*}} = getelementptr inbounds [1 x %struct.__va_list_tag], ptr %{{.*}} // OGCG: call void @llvm.va_copy.p0(ptr %{{.*}}, ptr %{{.*}} + +// Test handling where the first argument is not a count, as permitted by C23. +int varargs_new(char *fmt, ...) { + __builtin_va_list args; + __builtin_va_start(args, fmt); + int res = __builtin_va_arg(args, int); + __builtin_va_end(args); + return res; +} + +// CIR-LABEL: cir.func {{.*}} @varargs_new( +// CIR: %[[RET_ADDR:.+]] = cir.alloca !s32i, !cir.ptr<!s32i>, ["__retval"] +// CIR: %[[VAAREA:.+]] = cir.alloca !cir.array<!rec___va_list_tag x 1>, !cir.ptr<!cir.array<!rec___va_list_tag x 1>>, ["args"] +// CIR: %[[RES_ADDR:.+]] = cir.alloca !s32i, !cir.ptr<!s32i>, ["res", init] +// CIR: %[[VA_PTR0:.+]] = cir.cast array_to_ptrdecay %[[VAAREA]] : !cir.ptr<!cir.array<!rec___va_list_tag x 1>> -> !cir.ptr<!rec___va_list_tag> +// CIR: cir.va_start %[[VA_PTR0]] : !cir.ptr<!rec___va_list_tag> +// CIR: %[[VA_PTR1:.+]] = cir.cast array_to_ptrdecay %[[VAAREA]] : !cir.ptr<!cir.array<!rec___va_list_tag x 1>> -> !cir.ptr<!rec___va_list_tag> +// CIR: %[[VA_ARG:.+]] = cir.va_arg %[[VA_PTR1]] : (!cir.ptr<!rec___va_list_tag>) -> !s32i +// CIR: cir.store{{.*}} %[[VA_ARG]], %[[RES_ADDR]] : !s32i, !cir.ptr<!s32i> +// CIR: %[[VA_PTR2:.+]] = cir.cast array_to_ptrdecay %[[VAAREA]] : !cir.ptr<!cir.array<!rec___va_list_tag x 1>> -> !cir.ptr<!rec___va_list_tag> +// CIR: cir.va_end %[[VA_PTR2]] : !cir.ptr<!rec___va_list_tag> +// CIR: %[[RESULT:.+]] = cir.load{{.*}} %[[RES_ADDR]] : !cir.ptr<!s32i>, !s32i +// CIR: cir.store %[[RESULT]], %[[RET_ADDR]] : !s32i, !cir.ptr<!s32i> +// CIR: %[[RETVAL:.+]] = cir.load{{.*}} %[[RET_ADDR]] : !cir.ptr<!s32i>, !s32i +// CIR: cir.return %[[RETVAL]] : !s32i + +// LLVM-LABEL: define dso_local i32 @varargs_new( +// LLVM: %[[FMT_ADDR:.+]] = alloca ptr +// LLVM: %[[RET_ADDR:.+]] = alloca i32{{.*}} +// LLVM: %[[VAAREA:.+]] = alloca [1 x %struct.__va_list_tag]{{.*}} +// LLVM: %[[RES_ADDR:.+]] = alloca i32{{.*}} +// LLVM: %[[VA_PTR0:.+]] = getelementptr %struct.__va_list_tag, ptr %[[VAAREA]], i32 0 +// LLVM: call void @llvm.va_start.p0(ptr %[[VA_PTR0]]) +// LLVM: %[[VA_PTR1:.+]] = getelementptr %struct.__va_list_tag, ptr %[[VAAREA]], i32 0 +// LLVM: %[[VA_ARG:.+]] = va_arg ptr %[[VA_PTR1]], i32 +// LLVM: store i32 %[[VA_ARG]], ptr %[[RES_ADDR]], {{.*}} +// LLVM: %[[VA_PTR2:.+]] = getelementptr %struct.__va_list_tag, ptr %[[VAAREA]], i32 0 +// LLVM: call void @llvm.va_end.p0(ptr %[[VA_PTR2]]) +// LLVM: %[[TMP_LOAD:.+]] = load i32, ptr %[[RES_ADDR]], {{.*}} +// LLVM: store i32 %[[TMP_LOAD]], ptr %[[RET_ADDR]], {{.*}} +// LLVM: %[[RETVAL:.+]] = load i32, ptr %[[RET_ADDR]], {{.*}} +// LLVM: ret i32 %[[RETVAL]] + +// OGCG-LABEL: define dso_local i32 @varargs_new +// OGCG: %[[FMT_ADDR:.+]] = alloca ptr +// OGCG: %[[VAAREA:.+]] = alloca [1 x %struct.__va_list_tag] +// OGCG: %[[RES_ADDR:.+]] = alloca i32 +// OGCG: %[[DECAY:.+]] = getelementptr inbounds [1 x %struct.__va_list_tag], ptr %[[VAAREA]] +// OGCG: call void @llvm.va_start.p0(ptr %[[DECAY]]) +// OGCG: %[[DECAY1:.+]] = getelementptr inbounds [1 x %struct.__va_list_tag], ptr %[[VAAREA]] +// OGCG: %[[GPOFFSET_PTR:.+]] = getelementptr inbounds nuw %struct.__va_list_tag, ptr %[[DECAY1]], i32 0, i32 0 +// OGCG: %[[GPOFFSET:.+]] = load i32, ptr %[[GPOFFSET_PTR]] +// OGCG: %[[COND:.+]] = icmp ule i32 %[[GPOFFSET]], 40 +// OGCG: br i1 %[[COND]], label %vaarg.in_reg, label %vaarg.in_mem +// +// OGCG: vaarg.in_reg: +// OGCG: %[[REGSAVE_PTR:.+]] = getelementptr inbounds nuw %struct.__va_list_tag, ptr %[[DECAY1]], i32 0, i32 3 +// OGCG: %[[REGSAVE:.+]] = load ptr, ptr %[[REGSAVE_PTR]] +// OGCG: %[[VAADDR1:.+]] = getelementptr i8, ptr %[[REGSAVE]], i32 %[[GPOFFSET]] +// OGCG: br label %vaarg.end +// +// OGCG: vaarg.in_mem: +// OGCG: %[[OVERFLOW_PTR:.+]] = getelementptr inbounds nuw %struct.__va_list_tag, ptr %[[DECAY1]], i32 0, i32 2 +// OGCG: %[[OVERFLOW:.+]] = load ptr, ptr %[[OVERFLOW_PTR]] +// OGCG: br label %vaarg.end +// +// OGCG: vaarg.end: +// OGCG: %[[PHI:.+]] = phi ptr [ %[[VAADDR1]], %vaarg.in_reg ], [ %[[OVERFLOW]], %vaarg.in_mem ] +// OGCG: %[[LOADED:.+]] = load i32, ptr %[[PHI]] +// OGCG: store i32 %[[LOADED]], ptr %[[RES_ADDR]] +// OGCG: %[[DECAY2:.+]] = getelementptr inbounds [1 x %struct.__va_list_tag], ptr %[[VAAREA]] +// OGCG: call void @llvm.va_end.p0(ptr %[[DECAY2]]) +// OGCG: %[[VAL:.+]] = load i32, ptr %[[RES_ADDR]] +// OGCG: ret i32 %[[VAL]] _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
