From eb5e1ea417c8753d82db4ed90e89377ae549521e Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Tue, 11 Feb 2025 16:28:45 +0100 Subject: [PATCH] [MLIR][LLVMIR] Use TargetFolder when creating globals The LLVM dialect lowers globals using IRBuilder, relying on it creating constant expressions where possible. As we remove support for more constant expressions (per https://discourse.llvm.org/t/rfc-remove-most-constant-expressions/63179), this can cause issues for cases where the constant expression is no longer supported, and the operand cannot be constant folded without DataLayout being available. In particular, I ran into this issue with flang and the removal of mul constant expressions. Address this by using TargetFolder when creating globals, which will perform DL-aware constant folding. I think it would make sense to also do this in general, but I'm starting with globals where not doing this can result in translation failures. Ideally, globals with these problematic expressions would never be generated in the first place, but there has been essentially no movemenet on fixing this (https://github.com/llvm/llvm-project/issues/96047). --- flang/test/Fir/box.fir | 4 ++-- flang/test/Fir/rebox-global.fir | 2 +- flang/test/Fir/type-descriptor.fir | 4 ++-- mlir/lib/Target/LLVMIR/ModuleTranslation.cpp | 9 +++++++-- mlir/test/Target/LLVMIR/llvmir.mlir | 2 +- .../LLVMIR/omptarget-declare-target-llvm-host.mlir | 2 +- 6 files changed, 14 insertions(+), 9 deletions(-) diff --git a/flang/test/Fir/box.fir b/flang/test/Fir/box.fir index d4a5157888367..ef6be4d9b513e 100644 --- a/flang/test/Fir/box.fir +++ b/flang/test/Fir/box.fir @@ -1,7 +1,7 @@ // RUN: tco -o - %s | FileCheck %s // Global box initialization (test must come first because llvm globals are emitted first). -// CHECK-LABEL: @globalx = internal global { ptr, i64, i32, i8, i8, i8, i8 } { ptr null, i64 ptrtoint (ptr getelementptr (i32, ptr null, i32 1) to i64), i32 20240719, i8 0, i8 9, i8 2, i8 0 } +// CHECK-LABEL: @globalx = internal global { ptr, i64, i32, i8, i8, i8, i8 } { ptr null, i64 4, i32 20240719, i8 0, i8 9, i8 2, i8 0 } fir.global internal @globalx : !fir.box> { %c0 = arith.constant 0 : index %0 = fir.convert %c0 : (index) -> !fir.heap @@ -9,7 +9,7 @@ fir.global internal @globalx : !fir.box> { fir.has_value %1 : !fir.box> } -// CHECK-LABEL: @globaly = internal global { ptr, i64, i32, i8, i8, i8, i8, [1 x [3 x i64]] } { ptr null, i64 ptrtoint (ptr getelementptr (float, ptr null, i32 1) to i64), i32 20240719, i8 1, i8 27, i8 2, i8 0,{{.*}}[3 x i64] [i64 1, i64 0, i64 ptrtoint (ptr getelementptr (float, ptr null, i32 1) to i64)] +// CHECK-LABEL: @globaly = internal global { ptr, i64, i32, i8, i8, i8, i8, [1 x [3 x i64]] } { ptr null, i64 4, i32 20240719, i8 1, i8 27, i8 2, i8 0,{{.*}}[3 x i64] [i64 1, i64 0, i64 4] fir.global internal @globaly : !fir.box>> { %c0 = arith.constant 0 : index %0 = fir.convert %c0 : (index) -> !fir.heap> diff --git a/flang/test/Fir/rebox-global.fir b/flang/test/Fir/rebox-global.fir index 509487ab61d53..873adc123322c 100644 --- a/flang/test/Fir/rebox-global.fir +++ b/flang/test/Fir/rebox-global.fir @@ -20,4 +20,4 @@ fir.global @pointer_char4_init : !fir.box>> { fir.has_value %2 : !fir.box>> } // CHECK-LABEL: @pointer_char4_init -// CHECK-SAME: { ptr @char4, i64 ptrtoint (ptr getelementptr ([10 x i32], ptr null, i32 1) to i64), i32 20240719, i8 0, i8 44, i8 1, i8 0 } +// CHECK-SAME: { ptr @char4, i64 40, i32 20240719, i8 0, i8 44, i8 1, i8 0 } diff --git a/flang/test/Fir/type-descriptor.fir b/flang/test/Fir/type-descriptor.fir index 3b58a2f68251a..ab48caeb4d199 100644 --- a/flang/test/Fir/type-descriptor.fir +++ b/flang/test/Fir/type-descriptor.fir @@ -13,7 +13,7 @@ fir.global internal @_QFfooEx : !fir.box> { fir.has_value %1 : !fir.box> } // CHECK: @_QFfooEx = internal global { ptr, i64, i32, i8, i8, i8, i8, ptr, [1 x i64] } -// CHECK-SAME: { ptr null, i64 ptrtoint (ptr getelementptr (%_QFfooTsometype, ptr null, i32 1) to i64), +// CHECK-SAME: { ptr null, i64 80, // CHECK-SAME: i32 20240719, i8 0, i8 42, i8 2, i8 1, ptr @_QFfooEXdtXsometype, [1 x i64] zeroinitializer } !some_pdt_type = !fir.type<_QFfooTsome_pdt_typeK42K43{num:i32,values:!fir.box>>}> @@ -25,5 +25,5 @@ fir.global internal @_QFfooEx2 : !fir.box> { fir.has_value %1 : !fir.box> } // CHECK: @_QFfooEx2 = internal global { ptr, i64, i32, i8, i8, i8, i8, ptr, [1 x i64] } -// CHECK-SAME: { ptr null, i64 ptrtoint (ptr getelementptr (%_QFfooTsome_pdt_typeK42K43, ptr null, i32 1) to i64), +// CHECK-SAME: { ptr null, i64 80, // CHECK-SAME: i32 20240719, i8 0, i8 42, i8 2, i8 1, ptr @_QFfooEXdtXsome_pdt_typeX42X43, [1 x i64] zeroinitializer } diff --git a/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp b/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp index ed61cb255be8f..caea4ca44d143 100644 --- a/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp +++ b/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp @@ -38,6 +38,7 @@ #include "llvm/ADT/SetVector.h" #include "llvm/ADT/StringExtras.h" #include "llvm/ADT/TypeSwitch.h" +#include "llvm/Analysis/TargetFolder.h" #include "llvm/Frontend/OpenMP/OMPIRBuilder.h" #include "llvm/IR/BasicBlock.h" #include "llvm/IR/CFG.h" @@ -1171,7 +1172,9 @@ LogicalResult ModuleTranslation::convertGlobalsAndAliases() { // Convert global variable bodies. for (auto op : getModuleBody(mlirModule).getOps()) { if (Block *initializer = op.getInitializerBlock()) { - llvm::IRBuilder<> builder(llvmModule->getContext()); + llvm::IRBuilder builder( + llvmModule->getContext(), + llvm::TargetFolder(llvmModule->getDataLayout())); [[maybe_unused]] int numConstantsHit = 0; [[maybe_unused]] int numConstantsErased = 0; @@ -1282,7 +1285,9 @@ LogicalResult ModuleTranslation::convertGlobalsAndAliases() { // Convert global alias bodies. for (auto op : getModuleBody(mlirModule).getOps()) { Block &initializer = op.getInitializerBlock(); - llvm::IRBuilder<> builder(llvmModule->getContext()); + llvm::IRBuilder builder( + llvmModule->getContext(), + llvm::TargetFolder(llvmModule->getDataLayout())); for (mlir::Operation &op : initializer.without_terminator()) { if (failed(convertOperation(op, builder))) diff --git a/mlir/test/Target/LLVMIR/llvmir.mlir b/mlir/test/Target/LLVMIR/llvmir.mlir index 52aa69f4c481f..c0b3a9cb43022 100644 --- a/mlir/test/Target/LLVMIR/llvmir.mlir +++ b/mlir/test/Target/LLVMIR/llvmir.mlir @@ -84,7 +84,7 @@ llvm.mlir.global external @explicit_undef() : i32 { llvm.return %0 : i32 } -// CHECK: @int_gep = internal constant ptr getelementptr (i32, ptr @i32_global, i32 2) +// CHECK: @int_gep = internal constant ptr getelementptr (i8, ptr @i32_global, i64 8) llvm.mlir.global internal constant @int_gep() : !llvm.ptr { %addr = llvm.mlir.addressof @i32_global : !llvm.ptr %_c0 = llvm.mlir.constant(2: i32) : i32 diff --git a/mlir/test/Target/LLVMIR/omptarget-declare-target-llvm-host.mlir b/mlir/test/Target/LLVMIR/omptarget-declare-target-llvm-host.mlir index 0700c378b0056..92c85738dbc72 100644 --- a/mlir/test/Target/LLVMIR/omptarget-declare-target-llvm-host.mlir +++ b/mlir/test/Target/LLVMIR/omptarget-declare-target-llvm-host.mlir @@ -133,7 +133,7 @@ module attributes {llvm.target_triple = "x86_64-unknown-linux-gnu", omp.is_targe llvm.return %0 : i32 } - // CHECK-DAG: @_QMtest_0Ept1 = global { ptr, i64, i32, i8, i8, i8, i8 } { ptr null, i64 ptrtoint (ptr getelementptr (i32, ptr null, i32 1) to i64), i32 20180515, i8 0, i8 9, i8 1, i8 0 } + // CHECK-DAG: @_QMtest_0Ept1 = global { ptr, i64, i32, i8, i8, i8, i8 } { ptr null, i64 4, i32 20180515, i8 0, i8 9, i8 1, i8 0 } // CHECK-DAG: @_QMtest_0Ept1_decl_tgt_ref_ptr = weak global ptr @_QMtest_0Ept1 // CHECK-DAG: @.offloading.entry_name{{.*}} = internal unnamed_addr constant [31 x i8] c"_QMtest_0Ept1_decl_tgt_ref_ptr\00" // CHECK-DAG: @.offloading.entry._QMtest_0Ept1_decl_tgt_ref_ptr = weak constant %struct.__tgt_offload_entry { i64 0, i16 1, i16 1, i32 1, ptr @_QMtest_0Ept1_decl_tgt_ref_ptr, ptr @.offloading.entry_name{{.*}}, i64 8, i64 0, ptr null }, section "llvm_offload_entries"