Add Function Argument Demotion support #170915
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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.
adams381
requested review from
andykaylor,
bcardosolopes,
lanza and
xlauko
as code owners
llvmbot
added
clang
Member
|
@llvm/pr-subscribers-clangir Author: None (adams381) ChangesThis PR 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
Tested: All CIR tests pass (320/321, 99.69%). The one unsupported test is an expected failure. Full diff: https://github.com/llvm/llvm-project/pull/170915.diff 3 Files Affected:
diff --git a/clang/lib/CIR/CodeGen/CIRGenFunction.cpp b/clang/lib/CIR/CodeGen/CIRGenFunction.cpp
index 5d5209b9ffb60..10c4a16fb9687 100644
--- a/clang/lib/CIR/CodeGen/CIRGenFunction.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenFunction.cpp
@@ -412,28 +412,30 @@ void CIRGenFunction::LexicalScope::emitImplicitReturn() {
(void)emitReturn(localScope->endLoc);
}
-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<FunctionDecl>(d);
- curFuncDecl = d->getNonClosureContext();
+ assert((mlir::isa<cir::IntType>(ty) || cir::isAnyFloatingPointType(ty)) &&
+ "unexpected promotion type");
- prologueCleanupDepth = ehStack.stable_begin();
+ if (mlir::isa<cir::IntType>(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);
@@ -456,7 +458,7 @@ void CIRGenFunction::startFunction(GlobalDecl gd, QualType returnType,
cast<ParmVarDecl>(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.
@@ -464,6 +466,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<FunctionDecl>(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 e5cecaa573a6e..7ce7998825b18 100644
--- a/clang/lib/CIR/CodeGen/CIRGenFunction.h
+++ b/clang/lib/CIR/CodeGen/CIRGenFunction.h
@@ -901,6 +901,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<!s16i>, ["x", init]
+// CHECK: %1 = cir.cast integral %arg0 : !s32i -> !s16i
+// CHECK: cir.store %1, %0 : !s16i, !cir.ptr<!s16i>
+void foo(x) short x; {}
+
+// CHECK: cir.func no_proto dso_local @bar(%arg0: !cir.double
+// CHECK: %0 = cir.alloca !cir.float, !cir.ptr<!cir.float>, ["f", init]
+// CHECK: %1 = cir.cast floating %arg0 : !cir.double -> !cir.float
+// CHECK: cir.store %1, %0 : !cir.float, !cir.ptr<!cir.float>
+void bar(f) float f; {}
|
Member
|
@llvm/pr-subscribers-clang Author: None (adams381) ChangesThis PR 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
Tested: All CIR tests pass (320/321, 99.69%). The one unsupported test is an expected failure. Full diff: https://github.com/llvm/llvm-project/pull/170915.diff 3 Files Affected:
diff --git a/clang/lib/CIR/CodeGen/CIRGenFunction.cpp b/clang/lib/CIR/CodeGen/CIRGenFunction.cpp
index 5d5209b9ffb60..10c4a16fb9687 100644
--- a/clang/lib/CIR/CodeGen/CIRGenFunction.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenFunction.cpp
@@ -412,28 +412,30 @@ void CIRGenFunction::LexicalScope::emitImplicitReturn() {
(void)emitReturn(localScope->endLoc);
}
-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<FunctionDecl>(d);
- curFuncDecl = d->getNonClosureContext();
+ assert((mlir::isa<cir::IntType>(ty) || cir::isAnyFloatingPointType(ty)) &&
+ "unexpected promotion type");
- prologueCleanupDepth = ehStack.stable_begin();
+ if (mlir::isa<cir::IntType>(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);
@@ -456,7 +458,7 @@ void CIRGenFunction::startFunction(GlobalDecl gd, QualType returnType,
cast<ParmVarDecl>(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.
@@ -464,6 +466,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<FunctionDecl>(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 e5cecaa573a6e..7ce7998825b18 100644
--- a/clang/lib/CIR/CodeGen/CIRGenFunction.h
+++ b/clang/lib/CIR/CodeGen/CIRGenFunction.h
@@ -901,6 +901,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<!s16i>, ["x", init]
+// CHECK: %1 = cir.cast integral %arg0 : !s32i -> !s16i
+// CHECK: cir.store %1, %0 : !s16i, !cir.ptr<!s16i>
+void foo(x) short x; {}
+
+// CHECK: cir.func no_proto dso_local @bar(%arg0: !cir.double
+// CHECK: %0 = cir.alloca !cir.float, !cir.ptr<!cir.float>, ["f", init]
+// CHECK: %1 = cir.cast floating %arg0 : !cir.double -> !cir.float
+// CHECK: cir.store %1, %0 : !cir.float, !cir.ptr<!cir.float>
+void bar(f) float f; {}
|
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).
andykaylor
reviewed
Dec 5, 2025
Contributor
andykaylor
left a comment
andykaylor
left a comment
There was a problem hiding this comment.
This looks good. I have just a couple of nits and a test request.
Comment on lines
+432
to
+433
| return cgf.getBuilder().CIRBaseBuilderTy::createCast(cir::CastKind::floating, | ||
| value, ty); |
Contributor
There was a problem hiding this comment.
Suggested change
| 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) { | ||
| // Declare all the function arguments in the symbol table. |
Contributor
There was a problem hiding this comment.
Classic codegen starts this function by checking the FunctionDecl for NakedAttr and returning immediately if it's present. We should have a check for that.
// Naked functions don't have prologues.
if (fd && fd->hasAttr<NakedAttr>()) {
cgm.errorNYI(bodyBeginLoc, "naked function decl");
}
| @@ -0,0 +1,13 @@ | |||
| // RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fclangir -emit-cir %s -o - | FileCheck %s | |||
Contributor
There was a problem hiding this comment.
Can you add LLVM and OGCG checks for this test?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR 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
Tested: All CIR tests pass (320/321, 99.69%). The one unsupported test is an expected failure.