Skip to content

[CIR][Lowering] Core dialects: passthrough symbol visibility #1620

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

[CIR][Lowering] Core dialects: passthrough symbol visibility #1620

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
e69f483
passing through sym_visibility, arg_attrs and res_attrs for cir.func …
felixdaas May 16, 2025
40cb63d
added coversion of CIR TargetTripleAttr to llvm mlir TargetTripleAttr…
felixdaas May 16, 2025
280bb46
added two testcases to test if 1. the sym_visibility attr is passed c…
felixdaas May 21, 2025
451ce56
Simplified tests, added --triple option to declaration test + added c…
felixdaas May 22, 2025
71a99b8
removed the passing of arg_attrs and res_attrs + adjusted the tests
felixdaas May 22, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion clang/lib/CIR/Lowering/ThroughMLIR/LowerCIRToMLIR.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -540,13 +540,20 @@ class CIRFuncOpLowering : public mlir::OpConversionPattern<cir::FuncOp> {
signatureConversion.addInputs(argType.index(), convertedType);
}

SmallVector<mlir::NamedAttribute, 2> passThroughAttrs;

if (auto symVisibilityAttr = op.getSymVisibilityAttr())
passThroughAttrs.push_back(
rewriter.getNamedAttr("sym_visibility", symVisibilityAttr));

mlir::Type resultType =
getTypeConverter()->convertType(fnType.getReturnType());
auto fn = rewriter.create<mlir::func::FuncOp>(
op.getLoc(), op.getName(),
rewriter.getFunctionType(signatureConversion.getConvertedTypes(),
resultType ? mlir::TypeRange(resultType)
: mlir::TypeRange()));
: mlir::TypeRange()),
passThroughAttrs);

if (failed(rewriter.convertRegionTypes(&op.getBody(), *typeConverter,
&signatureConversion)))
Expand Down
12 changes: 12 additions & 0 deletions clang/lib/CIR/Lowering/ThroughMLIR/LowerMLIRToLLVM.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
#include "mlir/Pass/PassManager.h"
#include "mlir/Target/LLVMIR/Dialect/LLVMIR/LLVMToLLVMIRTranslation.h"
#include "mlir/Transforms/DialectConversion.h"
#include "clang/CIR/Dialect/IR/CIRDialect.h"
#include "clang/CIR/Passes.h"

using namespace cir;
Expand Down Expand Up @@ -62,6 +63,17 @@ void ConvertMLIRToLLVMPass::runOnOperation() {
populateFuncToLLVMConversionPatterns(typeConverter, patterns);

auto module = getOperation();

// Lower the module attributes to LLVM equivalents.
if (auto tripleAttr = module->getAttr(cir::CIRDialect::getTripleAttrName()))
module->setAttr(mlir::LLVM::LLVMDialect::getTargetTripleAttrName(),
tripleAttr);

// Strip the CIR attributes.
module->removeAttr(cir::CIRDialect::getSOBAttrName());
module->removeAttr(cir::CIRDialect::getLangAttrName());
module->removeAttr(cir::CIRDialect::getTripleAttrName());

if (failed(applyFullConversion(module, target, std::move(patterns))))
signalPassFailure();
}
Expand Down
9 changes: 9 additions & 0 deletions clang/test/CIR/Lowering/ThroughMLIR/function-attributes.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fclangir -fno-clangir-direct-lowering -emit-mlir=core %s -o %t.mlir
// RUN: FileCheck --input-file=%t.mlir %s

// CHECK: func.func private @declaration(i32) -> i32

int declaration(int x);
int declaration_test() {
return declaration(15);
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One more nit: code style requires all files to terminate with a newline,

1 change: 1 addition & 0 deletions clang/test/CIR/driver.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
// RUN: FileCheck --input-file=%t.cir %s -check-prefix=CIR_MACOS
// RUN: %clang -target arm64-apple-macosx12.0.0 -fclangir -S -emit-llvm %s -o %t3.ll
// RUN: FileCheck --input-file=%t3.ll %s -check-prefix=LLVM_MACOS
// RUN: %clang -target x86_64-unknown-linux-gnu -fclangir -fno-clangir-direct-lowering -c %s -o %t
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This path is already tested up there in the other -fno-clangir-direct-lowering invocation, turns out you don't need this test.

Copy link
Author

@felixdaas felixdaas May 23, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe I'm missing something here but I don't see where this is already tested. I't looks to me like all the other tests using the indirect lowering "only" create the cir,core dialects or llvm ir and never actually compile to an object file (the error occurs when trying to compile the llvm ir to an object file since during the mlir core to llvm ir translation the cir.triple attribute is not converted to the llvm.triple attribute and is thus discarded). If I try to add this line to the driver.c test and run the test locally (on ubuntu) with the current code on llvm/clangir:main I get the error mentioned in issue #1619

error: unable to create target: 'No available targets are compatible with triple ""'

If I run the test locally with the update code of this PR the test passes, so it seems to me like it does add value.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

True, thanks for clarifying! One last change, use -o %t.o


void foo(void) {}

Expand Down
Loading