From feee218c8e8fca15b57bd0798180ce57744bdce4 Mon Sep 17 00:00:00 2001 From: Adam Smith Date: Fri, 5 Dec 2025 10:26:19 -0800 Subject: [PATCH 1/5] Add Function Argument Demotion support This commit migrates the Function Argument Demotion feature from the incubator repository to upstream. The feature handles K&R-style function parameters that are promoted (e.g., short->int, float->double) and demotes them back to their declared types. Changes: - Add emitArgumentDemotion helper function for type demotion - Create emitFunctionProlog function to handle function prologue setup (addresses existing TODO to move parameter handling logic) - Move parameter handling logic into emitFunctionProlog - Add test case kr-func-promote.c to verify the feature Tested: All CIR tests pass (320/321, 99.69%). The one unsupported test is an expected failure. --- clang/lib/CIR/CodeGen/CIRGenFunction.cpp | 62 +++++++++++++++++------- clang/lib/CIR/CodeGen/CIRGenFunction.h | 4 ++ clang/test/CIR/CodeGen/kr-func-promote.c | 13 +++++ 3 files changed, 61 insertions(+), 18 deletions(-) create mode 100644 clang/test/CIR/CodeGen/kr-func-promote.c diff --git a/clang/lib/CIR/CodeGen/CIRGenFunction.cpp b/clang/lib/CIR/CodeGen/CIRGenFunction.cpp index 492cbb69fabb3..30e39729303e6 100644 --- a/clang/lib/CIR/CodeGen/CIRGenFunction.cpp +++ b/clang/lib/CIR/CodeGen/CIRGenFunction.cpp @@ -422,28 +422,30 @@ cir::TryOp CIRGenFunction::LexicalScope::getClosestTryParent() { return nullptr; } -void CIRGenFunction::startFunction(GlobalDecl gd, QualType returnType, - cir::FuncOp fn, cir::FuncType funcType, - FunctionArgList args, SourceLocation loc, - SourceLocation startLoc) { - assert(!curFn && - "CIRGenFunction can only be used for one function at a time"); +/// An argument came in as a promoted argument; demote it back to its +/// declared type. +static mlir::Value emitArgumentDemotion(CIRGenFunction &cgf, const VarDecl *var, + mlir::Value value) { + mlir::Type ty = cgf.convertType(var->getType()); - curFn = fn; + // This can happen with promotions that actually don't change the + // underlying type, like the enum promotions. + if (value.getType() == ty) + return value; - const Decl *d = gd.getDecl(); - - didCallStackSave = false; - curCodeDecl = d; - const auto *fd = dyn_cast_or_null(d); - curFuncDecl = d->getNonClosureContext(); + assert((mlir::isa(ty) || cir::isAnyFloatingPointType(ty)) && + "unexpected promotion type"); - prologueCleanupDepth = ehStack.stable_begin(); + if (mlir::isa(ty)) + return cgf.getBuilder().CIRBaseBuilderTy::createIntCast(value, ty); - mlir::Block *entryBB = &fn.getBlocks().front(); - builder.setInsertionPointToStart(entryBB); + return cgf.getBuilder().CIRBaseBuilderTy::createCast(cir::CastKind::floating, + value, ty); +} - // TODO(cir): this should live in `emitFunctionProlog +void CIRGenFunction::emitFunctionProlog(const FunctionArgList &args, + mlir::Block *entryBB, + const FunctionDecl *fd) { // Declare all the function arguments in the symbol table. for (const auto nameValue : llvm::zip(args, entryBB->getArguments())) { const VarDecl *paramVar = std::get<0>(nameValue); @@ -466,7 +468,7 @@ void CIRGenFunction::startFunction(GlobalDecl gd, QualType returnType, cast(paramVar)->isKNRPromoted(); assert(!cir::MissingFeatures::constructABIArgDirectExtend()); if (isPromoted) - cgm.errorNYI(fd->getSourceRange(), "Function argument demotion"); + paramVal = emitArgumentDemotion(*this, paramVar, paramVal); // Location of the store to the param storage tracked as beginning of // the function body. @@ -474,6 +476,30 @@ void CIRGenFunction::startFunction(GlobalDecl gd, QualType returnType, builder.CIRBaseBuilderTy::createStore(fnBodyBegin, paramVal, addrVal); } assert(builder.getInsertionBlock() && "Should be valid"); +} + +void CIRGenFunction::startFunction(GlobalDecl gd, QualType returnType, + cir::FuncOp fn, cir::FuncType funcType, + FunctionArgList args, SourceLocation loc, + SourceLocation startLoc) { + assert(!curFn && + "CIRGenFunction can only be used for one function at a time"); + + curFn = fn; + + const Decl *d = gd.getDecl(); + + didCallStackSave = false; + curCodeDecl = d; + const auto *fd = dyn_cast_or_null(d); + curFuncDecl = d->getNonClosureContext(); + + prologueCleanupDepth = ehStack.stable_begin(); + + mlir::Block *entryBB = &fn.getBlocks().front(); + builder.setInsertionPointToStart(entryBB); + + emitFunctionProlog(args, entryBB, fd); // When the current function is not void, create an address to store the // result value. diff --git a/clang/lib/CIR/CodeGen/CIRGenFunction.h b/clang/lib/CIR/CodeGen/CIRGenFunction.h index 0df812bcfb94e..05e33a631f390 100644 --- a/clang/lib/CIR/CodeGen/CIRGenFunction.h +++ b/clang/lib/CIR/CodeGen/CIRGenFunction.h @@ -954,6 +954,10 @@ class CIRGenFunction : public CIRGenTypeCache { clang::QualType buildFunctionArgList(clang::GlobalDecl gd, FunctionArgList &args); + /// Emit the function prologue: declare function arguments in the symbol table. + void emitFunctionProlog(const FunctionArgList &args, mlir::Block *entryBB, + const FunctionDecl *fd); + /// Emit code for the start of a function. /// \param loc The location to be associated with the function. /// \param startLoc The location of the function body. diff --git a/clang/test/CIR/CodeGen/kr-func-promote.c b/clang/test/CIR/CodeGen/kr-func-promote.c new file mode 100644 index 0000000000000..f34e11ce3e9e1 --- /dev/null +++ b/clang/test/CIR/CodeGen/kr-func-promote.c @@ -0,0 +1,13 @@ +// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fclangir -emit-cir %s -o - | FileCheck %s + +// CHECK: cir.func {{.*}}@foo(%arg0: !s32i +// CHECK: %0 = cir.alloca !s16i, !cir.ptr, ["x", init] +// CHECK: %1 = cir.cast integral %arg0 : !s32i -> !s16i +// CHECK: cir.store %1, %0 : !s16i, !cir.ptr +void foo(x) short x; {} + +// CHECK: cir.func no_proto dso_local @bar(%arg0: !cir.double +// CHECK: %0 = cir.alloca !cir.float, !cir.ptr, ["f", init] +// CHECK: %1 = cir.cast floating %arg0 : !cir.double -> !cir.float +// CHECK: cir.store %1, %0 : !cir.float, !cir.ptr +void bar(f) float f; {} From 00950197412b0eb37a1a850cb85eda3111758b4e Mon Sep 17 00:00:00 2001 From: Adam Smith Date: Fri, 5 Dec 2025 13:58:42 -0800 Subject: [PATCH 2/5] Fix null pointer dereference in emitFunctionProlog The emitFunctionProlog function was dereferencing fd->getBody() unconditionally without checking if fd is null or if getBody() returns null. Similarly, startFunction was dereferencing fd->getBody()->getEndLoc() without null checks. Fix by: - Adding SourceLocation bodyBeginLoc parameter to emitFunctionProlog - Computing bodyBeginLoc safely in startFunction with null checks - Computing bodyEndLoc safely in startFunction with null checks - Using fallback locations (startLoc/loc) when fd is null or has no body This prevents crashes when fd is null (e.g., for non-function declarations) or when the function has no body (declaration only). --- clang/lib/CIR/CodeGen/CIRGenFunction.cpp | 31 ++++++++++++++++++++---- clang/lib/CIR/CodeGen/CIRGenFunction.h | 2 +- 2 files changed, 27 insertions(+), 6 deletions(-) diff --git a/clang/lib/CIR/CodeGen/CIRGenFunction.cpp b/clang/lib/CIR/CodeGen/CIRGenFunction.cpp index 30e39729303e6..29613ca440029 100644 --- a/clang/lib/CIR/CodeGen/CIRGenFunction.cpp +++ b/clang/lib/CIR/CodeGen/CIRGenFunction.cpp @@ -445,7 +445,8 @@ static mlir::Value emitArgumentDemotion(CIRGenFunction &cgf, const VarDecl *var, void CIRGenFunction::emitFunctionProlog(const FunctionArgList &args, mlir::Block *entryBB, - const FunctionDecl *fd) { + const FunctionDecl *fd, + SourceLocation bodyBeginLoc) { // Declare all the function arguments in the symbol table. for (const auto nameValue : llvm::zip(args, entryBB->getArguments())) { const VarDecl *paramVar = std::get<0>(nameValue); @@ -472,7 +473,7 @@ void CIRGenFunction::emitFunctionProlog(const FunctionArgList &args, // Location of the store to the param storage tracked as beginning of // the function body. - mlir::Location fnBodyBegin = getLoc(fd->getBody()->getBeginLoc()); + mlir::Location fnBodyBegin = getLoc(bodyBeginLoc); builder.CIRBaseBuilderTy::createStore(fnBodyBegin, paramVal, addrVal); } assert(builder.getInsertionBlock() && "Should be valid"); @@ -499,13 +500,33 @@ void CIRGenFunction::startFunction(GlobalDecl gd, QualType returnType, mlir::Block *entryBB = &fn.getBlocks().front(); builder.setInsertionPointToStart(entryBB); - emitFunctionProlog(args, entryBB, fd); + // Determine the function body begin location for the prolog. + // If fd is null or has no body, use startLoc as fallback. + SourceLocation bodyBeginLoc = startLoc; + if (fd) { + if (Stmt *body = fd->getBody()) + bodyBeginLoc = body->getBeginLoc(); + else + bodyBeginLoc = fd->getLocation(); + } + + emitFunctionProlog(args, entryBB, fd, bodyBeginLoc); // When the current function is not void, create an address to store the // result value. - if (!returnType->isVoidType()) - emitAndUpdateRetAlloca(returnType, getLoc(fd->getBody()->getEndLoc()), + if (!returnType->isVoidType()) { + // Determine the function body end location. + // If fd is null or has no body, use loc as fallback. + SourceLocation bodyEndLoc = loc; + if (fd) { + if (Stmt *body = fd->getBody()) + bodyEndLoc = body->getEndLoc(); + else + bodyEndLoc = fd->getLocation(); + } + emitAndUpdateRetAlloca(returnType, getLoc(bodyEndLoc), getContext().getTypeAlignInChars(returnType)); + } if (isa_and_nonnull(d) && cast(d)->isInstance()) { diff --git a/clang/lib/CIR/CodeGen/CIRGenFunction.h b/clang/lib/CIR/CodeGen/CIRGenFunction.h index 05e33a631f390..73c63fedd531f 100644 --- a/clang/lib/CIR/CodeGen/CIRGenFunction.h +++ b/clang/lib/CIR/CodeGen/CIRGenFunction.h @@ -956,7 +956,7 @@ class CIRGenFunction : public CIRGenTypeCache { /// Emit the function prologue: declare function arguments in the symbol table. void emitFunctionProlog(const FunctionArgList &args, mlir::Block *entryBB, - const FunctionDecl *fd); + const FunctionDecl *fd, SourceLocation bodyBeginLoc); /// Emit code for the start of a function. /// \param loc The location to be associated with the function. From 681825128f50a3a2720b560eca271824d2db8537 Mon Sep 17 00:00:00 2001 From: Adam Smith Date: Tue, 9 Dec 2025 09:45:45 -0800 Subject: [PATCH 3/5] Address reviewer feedback for Function Argument Demotion PR - Use createFloatingCast instead of createCast with CastKind::floating - Add NakedAttr check in emitFunctionProlog to handle naked functions early - Add LLVM and OGCG checks to kr-func-promote.c test Addresses review comments from PR #170915 --- clang/lib/CIR/CodeGen/CIRGenFunction.cpp | 9 ++++- clang/test/CIR/CodeGen/kr-func-promote.c | 47 +++++++++++++++++++----- 2 files changed, 45 insertions(+), 11 deletions(-) diff --git a/clang/lib/CIR/CodeGen/CIRGenFunction.cpp b/clang/lib/CIR/CodeGen/CIRGenFunction.cpp index 29613ca440029..6b2e60a551198 100644 --- a/clang/lib/CIR/CodeGen/CIRGenFunction.cpp +++ b/clang/lib/CIR/CodeGen/CIRGenFunction.cpp @@ -16,6 +16,7 @@ #include "CIRGenCall.h" #include "CIRGenValue.h" #include "mlir/IR/Location.h" +#include "clang/AST/Attr.h" #include "clang/AST/ExprCXX.h" #include "clang/AST/GlobalDecl.h" #include "clang/CIR/MissingFeatures.h" @@ -439,14 +440,18 @@ static mlir::Value emitArgumentDemotion(CIRGenFunction &cgf, const VarDecl *var, if (mlir::isa(ty)) return cgf.getBuilder().CIRBaseBuilderTy::createIntCast(value, ty); - return cgf.getBuilder().CIRBaseBuilderTy::createCast(cir::CastKind::floating, - value, ty); + return cgf.getBuilder().createFloatingCast(value, ty); } void CIRGenFunction::emitFunctionProlog(const FunctionArgList &args, mlir::Block *entryBB, const FunctionDecl *fd, SourceLocation bodyBeginLoc) { + // Naked functions don't have prologues. + if (fd && fd->hasAttr()) { + cgm.errorNYI(bodyBeginLoc, "naked function decl"); + } + // Declare all the function arguments in the symbol table. for (const auto nameValue : llvm::zip(args, entryBB->getArguments())) { const VarDecl *paramVar = std::get<0>(nameValue); diff --git a/clang/test/CIR/CodeGen/kr-func-promote.c b/clang/test/CIR/CodeGen/kr-func-promote.c index f34e11ce3e9e1..d28e276859791 100644 --- a/clang/test/CIR/CodeGen/kr-func-promote.c +++ b/clang/test/CIR/CodeGen/kr-func-promote.c @@ -1,13 +1,42 @@ -// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fclangir -emit-cir %s -o - | FileCheck %s +// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -std=c89 -fclangir -emit-cir %s -o %t.cir +// RUN: FileCheck --input-file=%t.cir %s -check-prefix=CIR +// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -std=c89 -fclangir -emit-llvm %s -o %t-cir.ll +// RUN: FileCheck --input-file=%t-cir.ll %s -check-prefix=LLVM +// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -std=c89 -emit-llvm %s -o %t.ll +// RUN: FileCheck --input-file=%t.ll %s -check-prefix=OGCG -// CHECK: cir.func {{.*}}@foo(%arg0: !s32i -// CHECK: %0 = cir.alloca !s16i, !cir.ptr, ["x", init] -// CHECK: %1 = cir.cast integral %arg0 : !s32i -> !s16i -// CHECK: cir.store %1, %0 : !s16i, !cir.ptr +// CIR: cir.func {{.*}}@foo(%arg0: !s32i +// CIR: %0 = cir.alloca !s16i, !cir.ptr, ["x", init] +// CIR: %1 = cir.cast integral %arg0 : !s32i -> !s16i +// CIR: cir.store %1, %0 : !s16i, !cir.ptr +// expected-warning@+1 {{a function definition without a prototype is deprecated}} void foo(x) short x; {} -// CHECK: cir.func no_proto dso_local @bar(%arg0: !cir.double -// CHECK: %0 = cir.alloca !cir.float, !cir.ptr, ["f", init] -// CHECK: %1 = cir.cast floating %arg0 : !cir.double -> !cir.float -// CHECK: cir.store %1, %0 : !cir.float, !cir.ptr +// LLVM: define{{.*}} void @foo(i32 %0) +// LLVM: %[[X_PTR:.*]] = alloca i16, i64 1, align 2 +// LLVM: %[[X:.*]] = trunc i32 %0 to i16 +// LLVM: store i16 %[[X]], ptr %[[X_PTR]], align 2 + +// OGCG: define{{.*}} void @foo(i32 noundef %0) +// OGCG: entry: +// OGCG: %[[X_PTR:.*]] = alloca i16, align 2 +// OGCG: %[[X:.*]] = trunc i32 %0 to i16 +// OGCG: store i16 %[[X]], ptr %[[X_PTR]], align 2 + +// CIR: cir.func no_proto dso_local @bar(%arg0: !cir.double +// CIR: %0 = cir.alloca !cir.float, !cir.ptr, ["f", init] +// CIR: %1 = cir.cast floating %arg0 : !cir.double -> !cir.float +// CIR: cir.store %1, %0 : !cir.float, !cir.ptr +// expected-warning@+1 {{a function definition without a prototype is deprecated}} void bar(f) float f; {} + +// LLVM: define{{.*}} void @bar(double %0) +// LLVM: %[[F_PTR:.*]] = alloca float, i64 1, align 4 +// LLVM: %[[F:.*]] = fptrunc double %0 to float +// LLVM: store float %[[F]], ptr %[[F_PTR]], align 4 + +// OGCG: define{{.*}} void @bar(double noundef %0) +// OGCG: entry: +// OGCG: %[[F_PTR:.*]] = alloca float, align 4 +// OGCG: %[[F:.*]] = fptrunc double %0 to float +// OGCG: store float %[[F]], ptr %[[F_PTR]], align 4 From 72b64d869eed339bf1b78733a968376430fba9b5 Mon Sep 17 00:00:00 2001 From: Adam Smith Date: Tue, 9 Dec 2025 13:57:21 -0800 Subject: [PATCH 4/5] Fix formatting: wrap long comment line in CIRGenFunction.h --- clang/lib/CIR/CodeGen/CIRGenFunction.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/clang/lib/CIR/CodeGen/CIRGenFunction.h b/clang/lib/CIR/CodeGen/CIRGenFunction.h index 73c63fedd531f..15322ee72a1b0 100644 --- a/clang/lib/CIR/CodeGen/CIRGenFunction.h +++ b/clang/lib/CIR/CodeGen/CIRGenFunction.h @@ -954,7 +954,8 @@ class CIRGenFunction : public CIRGenTypeCache { clang::QualType buildFunctionArgList(clang::GlobalDecl gd, FunctionArgList &args); - /// Emit the function prologue: declare function arguments in the symbol table. + /// Emit the function prologue: declare function arguments in the symbol + /// table. void emitFunctionProlog(const FunctionArgList &args, mlir::Block *entryBB, const FunctionDecl *fd, SourceLocation bodyBeginLoc); From cbe65c25da2e2afc5b63e43ca8a9588f9a62cd37 Mon Sep 17 00:00:00 2001 From: Adam Smith Date: Tue, 9 Dec 2025 14:23:03 -0800 Subject: [PATCH 5/5] Fix test: make pattern flexible to handle optional no_inline attribute The no_inline attribute is added at -O0 (default), so the test pattern should be flexible to match with or without it. --- clang/test/CIR/CodeGen/kr-func-promote.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/test/CIR/CodeGen/kr-func-promote.c b/clang/test/CIR/CodeGen/kr-func-promote.c index d28e276859791..5fed068a30398 100644 --- a/clang/test/CIR/CodeGen/kr-func-promote.c +++ b/clang/test/CIR/CodeGen/kr-func-promote.c @@ -23,7 +23,7 @@ void foo(x) short x; {} // OGCG: %[[X:.*]] = trunc i32 %0 to i16 // OGCG: store i16 %[[X]], ptr %[[X_PTR]], align 2 -// CIR: cir.func no_proto dso_local @bar(%arg0: !cir.double +// CIR: cir.func{{.*}}no_proto dso_local @bar(%arg0: !cir.double // CIR: %0 = cir.alloca !cir.float, !cir.ptr, ["f", init] // CIR: %1 = cir.cast floating %arg0 : !cir.double -> !cir.float // CIR: cir.store %1, %0 : !cir.float, !cir.ptr