[CIR] Initial implementation of CIR-to-LLVM IR lowering pass #125260
Merged
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 change introduces lowering from CIR to LLVM IR of global integer variables, using defaults for attributes that aren't yet implemented.
andykaylor
requested review from
AaronBallman,
bcardosolopes,
erichkeane,
lanza and
dkolsen-pgi
llvmbot
added
clang
|
@llvm/pr-subscribers-clang Author: Andy Kaylor (andykaylor) ChangesThis change introduces lowering from CIR to LLVM IR of global integer variables, using defaults for attributes that aren't yet implemented. Full diff: https://github.com/llvm/llvm-project/pull/125260.diff 5 Files Affected:
diff --git a/clang/include/clang/CIR/LowerToLLVM.h b/clang/include/clang/CIR/LowerToLLVM.h
index afa1c1923ed516..1135dbcbe0354b 100644
--- a/clang/include/clang/CIR/LowerToLLVM.h
+++ b/clang/include/clang/CIR/LowerToLLVM.h
@@ -12,7 +12,9 @@
#ifndef CLANG_CIR_LOWERTOLLVM_H
#define CLANG_CIR_LOWERTOLLVM_H
-#include "mlir/Pass/Pass.h"
+#include "mlir/Dialect/LLVMIR/LLVMDialect.h"
+#include "mlir/Transforms/DialectConversion.h"
+#include "clang/CIR/Dialect/IR/CIRDialect.h"
#include <memory>
@@ -31,6 +33,24 @@ namespace direct {
std::unique_ptr<llvm::Module>
lowerDirectlyFromCIRToLLVMIR(mlir::ModuleOp mlirModule,
llvm::LLVMContext &llvmCtx);
+
+class CIRToLLVMGlobalOpLowering
+ : public mlir::OpConversionPattern<cir::GlobalOp> {
+ mlir::DataLayout const &dataLayout;
+
+public:
+ CIRToLLVMGlobalOpLowering(const mlir::TypeConverter &typeConverter,
+ mlir::MLIRContext *context,
+ mlir::DataLayout const &dataLayout)
+ : OpConversionPattern(typeConverter, context), dataLayout(dataLayout) {
+ setHasBoundedRewriteRecursion();
+ }
+
+ mlir::LogicalResult
+ matchAndRewrite(cir::GlobalOp op, OpAdaptor adaptor,
+ mlir::ConversionPatternRewriter &rewriter) const override;
+};
+
} // namespace direct
} // namespace cir
diff --git a/clang/lib/CIR/Lowering/DirectToLLVM/CMakeLists.txt b/clang/lib/CIR/Lowering/DirectToLLVM/CMakeLists.txt
index 0268234c3a2896..da802605676ac0 100644
--- a/clang/lib/CIR/Lowering/DirectToLLVM/CMakeLists.txt
+++ b/clang/lib/CIR/Lowering/DirectToLLVM/CMakeLists.txt
@@ -3,6 +3,13 @@ set(LLVM_LINK_COMPONENTS
Support
)
+get_property(dialect_libs GLOBAL PROPERTY MLIR_DIALECT_LIBS)
+
add_clang_library(clangCIRLoweringDirectToLLVM
LowerToLLVM.cpp
+
+ LINK_LIBS
+ ${dialect_libs}
+ MLIRBuiltinToLLVMIRTranslation
+ MLIRLLVMToLLVMIRTranslation
)
diff --git a/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp b/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp
index 63d2b51b428357..b24c41a93082d4 100644
--- a/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp
+++ b/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp
@@ -12,7 +12,19 @@
#include "clang/CIR/LowerToLLVM.h"
+#include "mlir/Conversion/LLVMCommon/TypeConverter.h"
+#include "mlir/Dialect/DLTI/DLTI.h"
+#include "mlir/Dialect/Func/IR/FuncOps.h"
+#include "mlir/Dialect/LLVMIR/LLVMDialect.h"
+#include "mlir/IR/BuiltinDialect.h"
#include "mlir/IR/BuiltinOps.h"
+#include "mlir/Pass/Pass.h"
+#include "mlir/Pass/PassManager.h"
+#include "mlir/Target/LLVMIR/Dialect/Builtin/BuiltinToLLVMIRTranslation.h"
+#include "mlir/Target/LLVMIR/Dialect/LLVMIR/LLVMToLLVMIRTranslation.h"
+#include "mlir/Target/LLVMIR/Export.h"
+#include "mlir/Transforms/DialectConversion.h"
+#include "clang/CIR/Dialect/IR/CIRDialect.h"
#include "llvm/IR/Module.h"
#include "llvm/Support/TimeProfiler.h"
@@ -22,13 +34,127 @@ using namespace llvm;
namespace cir {
namespace direct {
+struct ConvertCIRToLLVMPass
+ : public mlir::PassWrapper<ConvertCIRToLLVMPass,
+ mlir::OperationPass<mlir::ModuleOp>> {
+ void getDependentDialects(mlir::DialectRegistry ®istry) const override {
+ registry.insert<mlir::BuiltinDialect, mlir::DLTIDialect,
+ mlir::LLVM::LLVMDialect, mlir::func::FuncDialect>();
+ }
+ void runOnOperation() final;
+
+ StringRef getDescription() const override {
+ return "Convert the prepared CIR dialect module to LLVM dialect";
+ }
+
+ StringRef getArgument() const override { return "cir-flat-to-llvm"; }
+};
+
+mlir::LogicalResult CIRToLLVMGlobalOpLowering::matchAndRewrite(
+ cir::GlobalOp op, OpAdaptor adaptor,
+ mlir::ConversionPatternRewriter &rewriter) const {
+
+ // Fetch required values to create LLVM op.
+ const mlir::Type cirSymType = op.getSymType();
+
+ // This is the LLVM dialect type
+ const mlir::Type llvmType = getTypeConverter()->convertType(cirSymType);
+ // These defaults are just here until the equivalent attributes are
+ // available on cir.global ops.
+ const bool isConst = false;
+ const bool isDsoLocal = true;
+ const mlir::LLVM::Linkage linkage = mlir::LLVM::Linkage::External;
+ const StringRef symbol = op.getSymName();
+ std::optional<mlir::Attribute> init = op.getInitialValue();
+
+ SmallVector<mlir::NamedAttribute> attributes;
+
+ // Check for missing funcionalities.
+ if (!init.has_value()) {
+ rewriter.replaceOpWithNewOp<mlir::LLVM::GlobalOp>(
+ op, llvmType, isConst, linkage, symbol, mlir::Attribute(),
+ /*alignment*/ 0, /*addrSpace*/ 0, /*dsoLocal*/ isDsoLocal,
+ /*threadLocal*/ false, /*comdat*/ mlir::SymbolRefAttr(), attributes);
+ return mlir::success();
+ }
+
+ // Initializer is a constant array: convert it to a compatible llvm init.
+ if (auto intAttr = mlir::dyn_cast<cir::IntAttr>(init.value())) {
+ init = rewriter.getIntegerAttr(llvmType, intAttr.getValue());
+ } else {
+ op.emitError() << "unsupported initializer '" << init.value() << "'";
+ return mlir::failure();
+ }
+
+ // Rewrite op.
+ rewriter.replaceOpWithNewOp<mlir::LLVM::GlobalOp>(
+ op, llvmType, isConst, linkage, symbol, init.value(), /*alignment*/ 0,
+ /*addrSpace*/ 0, /*dsoLocal*/ isDsoLocal, /*threadLocal*/ false,
+ /*comdat*/ mlir::SymbolRefAttr(), attributes);
+
+ return mlir::success();
+}
+
+static void prepareTypeConverter(mlir::LLVMTypeConverter &converter,
+ mlir::DataLayout &dataLayout) {
+ converter.addConversion([&](cir::IntType type) -> mlir::Type {
+ // LLVM doesn't work with signed types, so we drop the CIR signs here.
+ return mlir::IntegerType::get(type.getContext(), type.getWidth());
+ });
+}
+
+void ConvertCIRToLLVMPass::runOnOperation() {
+ llvm::TimeTraceScope scope("Convert CIR to LLVM Pass");
+
+ mlir::ModuleOp module = getOperation();
+ mlir::DataLayout dl(module);
+ mlir::LLVMTypeConverter converter(&getContext());
+ prepareTypeConverter(converter, dl); // , lowerModule.get());
+
+ mlir::RewritePatternSet patterns(&getContext());
+
+ patterns.add<CIRToLLVMGlobalOpLowering>(converter, patterns.getContext(), dl);
+
+ mlir::ConversionTarget target(getContext());
+ target.addLegalOp<mlir::ModuleOp>();
+ target.addLegalDialect<mlir::LLVM::LLVMDialect>();
+ target.addIllegalDialect<mlir::BuiltinDialect, cir::CIRDialect,
+ mlir::func::FuncDialect>();
+
+ if (failed(applyPartialConversion(module, target, std::move(patterns))))
+ signalPassFailure();
+}
+
+static std::unique_ptr<mlir::Pass> createConvertCIRToLLVMPass() {
+ return std::make_unique<ConvertCIRToLLVMPass>();
+}
+
+static void populateCIRToLLVMPasses(mlir::OpPassManager &pm) {
+ pm.addPass(createConvertCIRToLLVMPass());
+}
+
std::unique_ptr<llvm::Module>
lowerDirectlyFromCIRToLLVMIR(mlir::ModuleOp mlirModule, LLVMContext &llvmCtx) {
llvm::TimeTraceScope scope("lower from CIR to LLVM directly");
+ mlir::MLIRContext *mlirCtx = mlirModule.getContext();
+
+ mlir::PassManager pm(mlirCtx);
+ populateCIRToLLVMPasses(pm);
+
+ bool result = !mlir::failed(pm.run(mlirModule));
+ if (!result)
+ report_fatal_error(
+ "The pass manager failed to lower CIR to LLVMIR dialect!");
+
+ mlir::registerBuiltinDialectTranslation(*mlirCtx);
+ mlir::registerLLVMDialectTranslation(*mlirCtx);
+
+ llvm::TimeTraceScope translateScope("translateModuleToLLVMIR");
+
std::optional<StringRef> moduleName = mlirModule.getName();
- auto llvmModule = std::make_unique<llvm::Module>(
- moduleName ? *moduleName : "CIRToLLVMModule", llvmCtx);
+ std::unique_ptr<llvm::Module> llvmModule = mlir::translateModuleToLLVMIR(
+ mlirModule, llvmCtx, moduleName ? *moduleName : "CIRToLLVMModule");
if (!llvmModule)
report_fatal_error("Lowering from LLVMIR dialect to llvm IR failed!");
diff --git a/clang/test/CIR/Lowering/global-var-simple.cpp b/clang/test/CIR/Lowering/global-var-simple.cpp
new file mode 100644
index 00000000000000..e8d2138de3f58b
--- /dev/null
+++ b/clang/test/CIR/Lowering/global-var-simple.cpp
@@ -0,0 +1,63 @@
+// Global variables of intergal types
+// RUN: %clang_cc1 -std=c++20 -triple x86_64-unknown-linux-gnu -fclangir -emit-llvm %s -o - | FileCheck %s
+
+// Note: Currently unsupported features include default zero-initialization
+// and alignment. The fact that "external" is only printed for globals
+// without an initializer is a quirk of the LLVM AsmWriter.
+
+char c;
+// CHECK: @c = external dso_local global i8
+
+signed char sc;
+// CHECK: @sc = external dso_local global i8
+
+unsigned char uc;
+// CHECK: @uc = external dso_local global i8
+
+short ss;
+// CHECK: @ss = external dso_local global i16
+
+unsigned short us = 100;
+// CHECK: @us = dso_local global i16 100
+
+int si = 42;
+// CHECK: @si = dso_local global i32 42
+
+unsigned ui;
+// CHECK: @ui = external dso_local global i32
+
+long sl;
+// CHECK: @sl = external dso_local global i64
+
+unsigned long ul;
+// CHECK: @ul = external dso_local global i64
+
+long long sll;
+// CHECK: @sll = external dso_local global i64
+
+unsigned long long ull = 123456;
+// CHECK: @ull = dso_local global i64 123456
+
+__int128 s128;
+// CHECK: @s128 = external dso_local global i128
+
+unsigned __int128 u128;
+// CHECK: @u128 = external dso_local global i128
+
+wchar_t wc;
+// CHECK: @wc = external dso_local global i32
+
+char8_t c8;
+// CHECK: @c8 = external dso_local global i8
+
+char16_t c16;
+// CHECK: @c16 = external dso_local global i16
+
+char32_t c32;
+// CHECK: @c32 = external dso_local global i32
+
+_BitInt(20) sb20;
+// CHECK: @sb20 = external dso_local global i20
+
+unsigned _BitInt(48) ub48;
+// CHECK: @ub48 = external dso_local global i48
diff --git a/clang/test/CIR/Lowering/hello.c b/clang/test/CIR/Lowering/hello.c
index 320041f0ab7dc9..ff78b6e6f6a5e2 100644
--- a/clang/test/CIR/Lowering/hello.c
+++ b/clang/test/CIR/Lowering/hello.c
@@ -1,8 +1,10 @@
// Smoke test for ClangIR-to-LLVM IR code generation
// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fclangir -emit-llvm %s -o - | FileCheck %s
-// TODO: Add checks when proper lowering is implemented.
-// For now, we're just creating an empty module.
-// CHECK: ModuleID
+int a;
-void foo() {}
+// CHECK: @a = external dso_local global i32
+
+int b = 2;
+
+// CHECK: @b = dso_local global i32 2
|
|
@llvm/pr-subscribers-clangir Author: Andy Kaylor (andykaylor) ChangesThis change introduces lowering from CIR to LLVM IR of global integer variables, using defaults for attributes that aren't yet implemented. Full diff: https://github.com/llvm/llvm-project/pull/125260.diff 5 Files Affected:
diff --git a/clang/include/clang/CIR/LowerToLLVM.h b/clang/include/clang/CIR/LowerToLLVM.h
index afa1c1923ed516..1135dbcbe0354b 100644
--- a/clang/include/clang/CIR/LowerToLLVM.h
+++ b/clang/include/clang/CIR/LowerToLLVM.h
@@ -12,7 +12,9 @@
#ifndef CLANG_CIR_LOWERTOLLVM_H
#define CLANG_CIR_LOWERTOLLVM_H
-#include "mlir/Pass/Pass.h"
+#include "mlir/Dialect/LLVMIR/LLVMDialect.h"
+#include "mlir/Transforms/DialectConversion.h"
+#include "clang/CIR/Dialect/IR/CIRDialect.h"
#include <memory>
@@ -31,6 +33,24 @@ namespace direct {
std::unique_ptr<llvm::Module>
lowerDirectlyFromCIRToLLVMIR(mlir::ModuleOp mlirModule,
llvm::LLVMContext &llvmCtx);
+
+class CIRToLLVMGlobalOpLowering
+ : public mlir::OpConversionPattern<cir::GlobalOp> {
+ mlir::DataLayout const &dataLayout;
+
+public:
+ CIRToLLVMGlobalOpLowering(const mlir::TypeConverter &typeConverter,
+ mlir::MLIRContext *context,
+ mlir::DataLayout const &dataLayout)
+ : OpConversionPattern(typeConverter, context), dataLayout(dataLayout) {
+ setHasBoundedRewriteRecursion();
+ }
+
+ mlir::LogicalResult
+ matchAndRewrite(cir::GlobalOp op, OpAdaptor adaptor,
+ mlir::ConversionPatternRewriter &rewriter) const override;
+};
+
} // namespace direct
} // namespace cir
diff --git a/clang/lib/CIR/Lowering/DirectToLLVM/CMakeLists.txt b/clang/lib/CIR/Lowering/DirectToLLVM/CMakeLists.txt
index 0268234c3a2896..da802605676ac0 100644
--- a/clang/lib/CIR/Lowering/DirectToLLVM/CMakeLists.txt
+++ b/clang/lib/CIR/Lowering/DirectToLLVM/CMakeLists.txt
@@ -3,6 +3,13 @@ set(LLVM_LINK_COMPONENTS
Support
)
+get_property(dialect_libs GLOBAL PROPERTY MLIR_DIALECT_LIBS)
+
add_clang_library(clangCIRLoweringDirectToLLVM
LowerToLLVM.cpp
+
+ LINK_LIBS
+ ${dialect_libs}
+ MLIRBuiltinToLLVMIRTranslation
+ MLIRLLVMToLLVMIRTranslation
)
diff --git a/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp b/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp
index 63d2b51b428357..b24c41a93082d4 100644
--- a/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp
+++ b/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp
@@ -12,7 +12,19 @@
#include "clang/CIR/LowerToLLVM.h"
+#include "mlir/Conversion/LLVMCommon/TypeConverter.h"
+#include "mlir/Dialect/DLTI/DLTI.h"
+#include "mlir/Dialect/Func/IR/FuncOps.h"
+#include "mlir/Dialect/LLVMIR/LLVMDialect.h"
+#include "mlir/IR/BuiltinDialect.h"
#include "mlir/IR/BuiltinOps.h"
+#include "mlir/Pass/Pass.h"
+#include "mlir/Pass/PassManager.h"
+#include "mlir/Target/LLVMIR/Dialect/Builtin/BuiltinToLLVMIRTranslation.h"
+#include "mlir/Target/LLVMIR/Dialect/LLVMIR/LLVMToLLVMIRTranslation.h"
+#include "mlir/Target/LLVMIR/Export.h"
+#include "mlir/Transforms/DialectConversion.h"
+#include "clang/CIR/Dialect/IR/CIRDialect.h"
#include "llvm/IR/Module.h"
#include "llvm/Support/TimeProfiler.h"
@@ -22,13 +34,127 @@ using namespace llvm;
namespace cir {
namespace direct {
+struct ConvertCIRToLLVMPass
+ : public mlir::PassWrapper<ConvertCIRToLLVMPass,
+ mlir::OperationPass<mlir::ModuleOp>> {
+ void getDependentDialects(mlir::DialectRegistry ®istry) const override {
+ registry.insert<mlir::BuiltinDialect, mlir::DLTIDialect,
+ mlir::LLVM::LLVMDialect, mlir::func::FuncDialect>();
+ }
+ void runOnOperation() final;
+
+ StringRef getDescription() const override {
+ return "Convert the prepared CIR dialect module to LLVM dialect";
+ }
+
+ StringRef getArgument() const override { return "cir-flat-to-llvm"; }
+};
+
+mlir::LogicalResult CIRToLLVMGlobalOpLowering::matchAndRewrite(
+ cir::GlobalOp op, OpAdaptor adaptor,
+ mlir::ConversionPatternRewriter &rewriter) const {
+
+ // Fetch required values to create LLVM op.
+ const mlir::Type cirSymType = op.getSymType();
+
+ // This is the LLVM dialect type
+ const mlir::Type llvmType = getTypeConverter()->convertType(cirSymType);
+ // These defaults are just here until the equivalent attributes are
+ // available on cir.global ops.
+ const bool isConst = false;
+ const bool isDsoLocal = true;
+ const mlir::LLVM::Linkage linkage = mlir::LLVM::Linkage::External;
+ const StringRef symbol = op.getSymName();
+ std::optional<mlir::Attribute> init = op.getInitialValue();
+
+ SmallVector<mlir::NamedAttribute> attributes;
+
+ // Check for missing funcionalities.
+ if (!init.has_value()) {
+ rewriter.replaceOpWithNewOp<mlir::LLVM::GlobalOp>(
+ op, llvmType, isConst, linkage, symbol, mlir::Attribute(),
+ /*alignment*/ 0, /*addrSpace*/ 0, /*dsoLocal*/ isDsoLocal,
+ /*threadLocal*/ false, /*comdat*/ mlir::SymbolRefAttr(), attributes);
+ return mlir::success();
+ }
+
+ // Initializer is a constant array: convert it to a compatible llvm init.
+ if (auto intAttr = mlir::dyn_cast<cir::IntAttr>(init.value())) {
+ init = rewriter.getIntegerAttr(llvmType, intAttr.getValue());
+ } else {
+ op.emitError() << "unsupported initializer '" << init.value() << "'";
+ return mlir::failure();
+ }
+
+ // Rewrite op.
+ rewriter.replaceOpWithNewOp<mlir::LLVM::GlobalOp>(
+ op, llvmType, isConst, linkage, symbol, init.value(), /*alignment*/ 0,
+ /*addrSpace*/ 0, /*dsoLocal*/ isDsoLocal, /*threadLocal*/ false,
+ /*comdat*/ mlir::SymbolRefAttr(), attributes);
+
+ return mlir::success();
+}
+
+static void prepareTypeConverter(mlir::LLVMTypeConverter &converter,
+ mlir::DataLayout &dataLayout) {
+ converter.addConversion([&](cir::IntType type) -> mlir::Type {
+ // LLVM doesn't work with signed types, so we drop the CIR signs here.
+ return mlir::IntegerType::get(type.getContext(), type.getWidth());
+ });
+}
+
+void ConvertCIRToLLVMPass::runOnOperation() {
+ llvm::TimeTraceScope scope("Convert CIR to LLVM Pass");
+
+ mlir::ModuleOp module = getOperation();
+ mlir::DataLayout dl(module);
+ mlir::LLVMTypeConverter converter(&getContext());
+ prepareTypeConverter(converter, dl); // , lowerModule.get());
+
+ mlir::RewritePatternSet patterns(&getContext());
+
+ patterns.add<CIRToLLVMGlobalOpLowering>(converter, patterns.getContext(), dl);
+
+ mlir::ConversionTarget target(getContext());
+ target.addLegalOp<mlir::ModuleOp>();
+ target.addLegalDialect<mlir::LLVM::LLVMDialect>();
+ target.addIllegalDialect<mlir::BuiltinDialect, cir::CIRDialect,
+ mlir::func::FuncDialect>();
+
+ if (failed(applyPartialConversion(module, target, std::move(patterns))))
+ signalPassFailure();
+}
+
+static std::unique_ptr<mlir::Pass> createConvertCIRToLLVMPass() {
+ return std::make_unique<ConvertCIRToLLVMPass>();
+}
+
+static void populateCIRToLLVMPasses(mlir::OpPassManager &pm) {
+ pm.addPass(createConvertCIRToLLVMPass());
+}
+
std::unique_ptr<llvm::Module>
lowerDirectlyFromCIRToLLVMIR(mlir::ModuleOp mlirModule, LLVMContext &llvmCtx) {
llvm::TimeTraceScope scope("lower from CIR to LLVM directly");
+ mlir::MLIRContext *mlirCtx = mlirModule.getContext();
+
+ mlir::PassManager pm(mlirCtx);
+ populateCIRToLLVMPasses(pm);
+
+ bool result = !mlir::failed(pm.run(mlirModule));
+ if (!result)
+ report_fatal_error(
+ "The pass manager failed to lower CIR to LLVMIR dialect!");
+
+ mlir::registerBuiltinDialectTranslation(*mlirCtx);
+ mlir::registerLLVMDialectTranslation(*mlirCtx);
+
+ llvm::TimeTraceScope translateScope("translateModuleToLLVMIR");
+
std::optional<StringRef> moduleName = mlirModule.getName();
- auto llvmModule = std::make_unique<llvm::Module>(
- moduleName ? *moduleName : "CIRToLLVMModule", llvmCtx);
+ std::unique_ptr<llvm::Module> llvmModule = mlir::translateModuleToLLVMIR(
+ mlirModule, llvmCtx, moduleName ? *moduleName : "CIRToLLVMModule");
if (!llvmModule)
report_fatal_error("Lowering from LLVMIR dialect to llvm IR failed!");
diff --git a/clang/test/CIR/Lowering/global-var-simple.cpp b/clang/test/CIR/Lowering/global-var-simple.cpp
new file mode 100644
index 00000000000000..e8d2138de3f58b
--- /dev/null
+++ b/clang/test/CIR/Lowering/global-var-simple.cpp
@@ -0,0 +1,63 @@
+// Global variables of intergal types
+// RUN: %clang_cc1 -std=c++20 -triple x86_64-unknown-linux-gnu -fclangir -emit-llvm %s -o - | FileCheck %s
+
+// Note: Currently unsupported features include default zero-initialization
+// and alignment. The fact that "external" is only printed for globals
+// without an initializer is a quirk of the LLVM AsmWriter.
+
+char c;
+// CHECK: @c = external dso_local global i8
+
+signed char sc;
+// CHECK: @sc = external dso_local global i8
+
+unsigned char uc;
+// CHECK: @uc = external dso_local global i8
+
+short ss;
+// CHECK: @ss = external dso_local global i16
+
+unsigned short us = 100;
+// CHECK: @us = dso_local global i16 100
+
+int si = 42;
+// CHECK: @si = dso_local global i32 42
+
+unsigned ui;
+// CHECK: @ui = external dso_local global i32
+
+long sl;
+// CHECK: @sl = external dso_local global i64
+
+unsigned long ul;
+// CHECK: @ul = external dso_local global i64
+
+long long sll;
+// CHECK: @sll = external dso_local global i64
+
+unsigned long long ull = 123456;
+// CHECK: @ull = dso_local global i64 123456
+
+__int128 s128;
+// CHECK: @s128 = external dso_local global i128
+
+unsigned __int128 u128;
+// CHECK: @u128 = external dso_local global i128
+
+wchar_t wc;
+// CHECK: @wc = external dso_local global i32
+
+char8_t c8;
+// CHECK: @c8 = external dso_local global i8
+
+char16_t c16;
+// CHECK: @c16 = external dso_local global i16
+
+char32_t c32;
+// CHECK: @c32 = external dso_local global i32
+
+_BitInt(20) sb20;
+// CHECK: @sb20 = external dso_local global i20
+
+unsigned _BitInt(48) ub48;
+// CHECK: @ub48 = external dso_local global i48
diff --git a/clang/test/CIR/Lowering/hello.c b/clang/test/CIR/Lowering/hello.c
index 320041f0ab7dc9..ff78b6e6f6a5e2 100644
--- a/clang/test/CIR/Lowering/hello.c
+++ b/clang/test/CIR/Lowering/hello.c
@@ -1,8 +1,10 @@
// Smoke test for ClangIR-to-LLVM IR code generation
// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fclangir -emit-llvm %s -o - | FileCheck %s
-// TODO: Add checks when proper lowering is implemented.
-// For now, we're just creating an empty module.
-// CHECK: ModuleID
+int a;
-void foo() {}
+// CHECK: @a = external dso_local global i32
+
+int b = 2;
+
+// CHECK: @b = dso_local global i32 2
|
erichkeane
reviewed
Jan 31, 2025
|
|
||
| class CIRToLLVMGlobalOpLowering | ||
| : public mlir::OpConversionPattern<cir::GlobalOp> { | ||
| mlir::DataLayout const &dataLayout; |
There was a problem hiding this comment.
curious why east const for only this type?
There was a problem hiding this comment.
I don't think I understand your question here.
There was a problem hiding this comment.
Typically we do const west, so const is on the left of the type. You do that everywhere else, except for this one point, where it is east const (on the right).
SO I guess I'm asking, why:
mlir::DataLayout const & instead of const mlir::DataLayout &? And only for this type (in the parameter list below as well).
There was a problem hiding this comment.
I see. I wasn't familiar with the east const terminology, thought maybe the "east" part was a typo.
I suspect this was just a style preference of the person who introduced this parameter in the incubator implementation. I'll make it consistent.
| return "Convert the prepared CIR dialect module to LLVM dialect"; | ||
| } | ||
|
|
||
| StringRef getArgument() const override { return "cir-flat-to-llvm"; } |
There was a problem hiding this comment.
what does 'flat' mean here?
There was a problem hiding this comment.
When control flow and scopes implemented in CIR, there will be nested regions describing that structure. When that happens another pass will be run before this one that flattens the CIR by inlining the nested regions to create a situation where all blocks in a function belong to the parent region.
Here's a simple example showing what that looks like: https://godbolt.org/z/Ta5dTMfPn
There was a problem hiding this comment.
Thanks! Wouldn't mind a comment somewhere explaining this is for 'flat' CIR only, as it is a bit of a sub-dialect it sounds?
There was a problem hiding this comment.
Yea, a comment might be good here since cir-flat hasn't been introduced just yet
| // Fetch required values to create LLVM op. | ||
| const mlir::Type cirSymType = op.getSymType(); | ||
|
|
||
| // This is the LLVM dialect type |
There was a problem hiding this comment.
Suggested change
| // This is the LLVM dialect type | |
| // This is the LLVM dialect type. |
|
|
||
| // This is the LLVM dialect type | ||
| const mlir::Type llvmType = getTypeConverter()->convertType(cirSymType); | ||
| // These defaults are just here until the equivalent attributes are |
There was a problem hiding this comment.
Suggested change
| // These defaults are just here until the equivalent attributes are | |
| // FIXME: These default values are placeholders until the the equivalent attributes are available on cir.global ops. |
^^ Plus word-wrap.
| // Rewrite op. | ||
| rewriter.replaceOpWithNewOp<mlir::LLVM::GlobalOp>( | ||
| op, llvmType, isConst, linkage, symbol, init.value(), /*alignment*/ 0, | ||
| /*addrSpace*/ 0, /*dsoLocal*/ isDsoLocal, /*threadLocal*/ false, |
There was a problem hiding this comment.
Suggested change
| /*addrSpace*/ 0, /*dsoLocal*/ isDsoLocal, /*threadLocal*/ false, | |
| /*addrSpace*/ 0, isDsoLocal, /*threadLocal*/ false, |
There was a problem hiding this comment.
Same other suggestions on comments.
| mlir::ModuleOp module = getOperation(); | ||
| mlir::DataLayout dl(module); | ||
| mlir::LLVMTypeConverter converter(&getContext()); | ||
| prepareTypeConverter(converter, dl); // , lowerModule.get()); |
There was a problem hiding this comment.
Suggested change
| prepareTypeConverter(converter, dl); // , lowerModule.get()); | |
| prepareTypeConverter(converter, dl); |
| mlir::PassManager pm(mlirCtx); | ||
| populateCIRToLLVMPasses(pm); | ||
|
|
||
| bool result = !mlir::failed(pm.run(mlirModule)); |
There was a problem hiding this comment.
This variable is harming readability to me i think. The name isn't clear which direction it means, plus the double-not is REALLY confusing here :D I suspect just if (mlir::failed(pm.run(mlirModule)) would be most readable.
There was a problem hiding this comment.
I expect reporting a fatal error here is also not what we want long term. I'll do something to clean it up.
| auto llvmModule = std::make_unique<llvm::Module>( | ||
| moduleName ? *moduleName : "CIRToLLVMModule", llvmCtx); | ||
| std::unique_ptr<llvm::Module> llvmModule = mlir::translateModuleToLLVMIR( | ||
| mlirModule, llvmCtx, moduleName ? *moduleName : "CIRToLLVMModule"); |
There was a problem hiding this comment.
Suggested change
| mlirModule, llvmCtx, moduleName ? *moduleName : "CIRToLLVMModule"); | |
| mlirModule, llvmCtx, moduleName.value_or("CIRToLLVMModule")); |
Better yet, make moduleName above a StringRef, and just do mlirModule.getName().value_or("CIRToLLVMModule"));
lanza
reviewed
Jan 31, 2025
| @@ -31,6 +33,24 @@ namespace direct { | |||
| std::unique_ptr<llvm::Module> | |||
| lowerDirectlyFromCIRToLLVMIR(mlir::ModuleOp mlirModule, | |||
| llvm::LLVMContext &llvmCtx); | |||
|
|
|||
| class CIRToLLVMGlobalOpLowering | |||
There was a problem hiding this comment.
This is originally in clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.h. Any reason you're promoting it to the include top level dir?
There was a problem hiding this comment.
I think that was an accident. This is a different file with the same name but different location. I think I missed that detail when I was moving this bit of code over from the incubator project.
erichkeane
approved these changes
Jan 31, 2025
|
|
||
| if (init.has_value()) { | ||
| if (auto fltAttr = mlir::dyn_cast<cir::FPAttr>(init.value())) { | ||
| // Initializer is a constant floating-point number: convert to MLIR |
There was a problem hiding this comment.
At a certain point, a type-visitor is going to be something we want for here, but ATM this is fine.
|
|
||
| static void prepareTypeConverter(mlir::LLVMTypeConverter &converter, | ||
| mlir::DataLayout &dataLayout) { | ||
| converter.addConversion([&](cir::IntType type) -> mlir::Type { |
There was a problem hiding this comment.
Its a shame there isn't an addConversions variadic version of this that we could just call all of these in 1 place. I see this comes from MLIR unfortunately, but perhaps something to suggest upstream elsewhere.
| StringRef getArgument() const override { return "cir-flat-to-llvm"; } | ||
| }; | ||
|
|
||
| // This pass requires the CIR to be in a "flat" state. All blocks in each |
There was a problem hiding this comment.
Would love more detail here, but I'm not knowledgable enough to review the text, so Bruno or Nathan can approve whatever they think is sufficiently descriptive.
There was a problem hiding this comment.
I'm not sure what more I can say here that won't reference things that don't exist yet.
@bcardosolopes @lanza Any suggestions?
There was a problem hiding this comment.
It doesn't necessarily have to reference only things in this upstream, enough that the future concept is more clear (your example helped, but I'm still only sorta-familiar with it), but enough so that I don't have to ask again next time I see it always saves us time.
There was a problem hiding this comment.
I'd actually just strip this comment away, probably comes from a time when we were incrementally transitioning to eliminate all regions in favor of basic blocks prior to lowering - right now in the incubator all of LLVM lowering assumes CIR to be flat, so this comment wouldn't be adding much.
There was a problem hiding this comment.
Actually, this is a comment I added in response to Erich's question about what "flat" meant in the pass argument string. It feels like there should be some way of enforcing this constraint, assuming we might eventually be in a situation where non-clang clients are pulling CIR passes into their own pipeline. That could eventually happen, right?
There was a problem hiding this comment.
Maybe move the comment closer to ConvertCIRToLLVMPass? It's a bit misleading to make it about one specific operation.
there should be some way of enforcing this constraint
In the case of GlobalOp, there are versions of it that might contain a region. Usually for other operations that always have regions (IfOp, SwitchOp, ScopeOp, etc), there's no lowering to LLVM available so you get a hard error if you'd somehow skip the flattening.
For GlobalOp, perhaps we could emit an error if ctorRegion or dtorRegion are not empty.
(Another mechanism we could use is to leverage legality: https://mlir.llvm.org/docs/DialectConversion/#conversion-target, but that doesn't help with GlobalOp, and for other ops we already get an error, so not sure it's viable)
we might eventually be in a situation where non-clang clients
I'd say a bit far-fetched given the dep on AST and other clang bits, but eventually, it's indeed possible.
There was a problem hiding this comment.
Oh! This was a total fail on my part. I intended for this comment to be above ConvertCIRToLLVMPass and I hadn't even noticed that I put it in the wrong place.
bcardosolopes
approved these changes
Jan 31, 2025
erichkeane
reviewed
Feb 3, 2025
| SmallVector<mlir::NamedAttribute> attributes; | ||
|
|
||
| if (init.has_value()) { | ||
| if (const auto fltAttr = mlir::dyn_cast<cir::FPAttr>(init.value())) { |
There was a problem hiding this comment.
Note, I THINK these need to be const auto * (assuming they are pointers).
There was a problem hiding this comment.
This is one of those things that's going to be counter-intuitive to people used to llvm::dyn_cast (it definitely was to me). These aren't pointers. It's related to the same thing we discussed with ModuleOp, where this is an object that wraps a pointer, so even though it's not a pointer, the object has bool and ! operators so it works in an if-statement like this.
I was going to suggest that I could spell out the type here rather than using auto, but I'm not sure that makes it less confusing and it definitely goes against standard mlir practice. Since this is in the mlir-specific part of the code, I think auto is the way to go here.
There was a problem hiding this comment.
This is one of those things that's going to be counter-intuitive to people used to
llvm::dyn_cast(it definitely was to me). These aren't pointers. It's related to the same thing we discussed withModuleOp, where this is an object that wraps a pointer, so even though it's not a pointer, the object hasbooland!operators so it works in an if-statement like this.I was going to suggest that I could spell out the type here rather than using
auto, but I'm not sure that makes it less confusing and it definitely goes against standard mlir practice. Since this is in the mlir-specific part of the code, I thinkautois the way to go here.
I don't have a problem with auto, it makes sense here as the RHS has the 'type' being cast to. Just you're right that mlir::dyn_cast not being a pointer type itself is interesting here/something I didn't know. Thanks for clarifying!
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/51/builds/10269 Here is the relevant piece of the build log for the reference |
Icohedron
pushed a commit
to Icohedron/llvm-project
that referenced
this pull request
Feb 11, 2025
…5260) This change introduces lowering from CIR to LLVM IR of global integer and floating-point variables, using defaults for attributes that aren't yet implemented.
bcardosolopes
pushed a commit
to bcardosolopes/llvm-project
that referenced
this pull request
Mar 6, 2025
…lvm#125260)" This reverts commit 622ee03.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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 change introduces lowering from CIR to LLVM IR of global integer variables, using defaults for attributes that aren't yet implemented.