[Mlir-commits] [flang] [mlir] [MLIR][LLVMIR] Use TargetFolder when creating globals (PR #126745)
llvmlistbot at llvm.org
llvmlistbot at llvm.org
Tue Feb 11 07:44:02 PST 2025
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-mlir-llvm
Author: Nikita Popov (nikic)
<details>
<summary>Changes</summary>
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 operation 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 little movement on fixing this (https://github.com/llvm/llvm-project/issues/96047).
---
Full diff: https://github.com/llvm/llvm-project/pull/126745.diff
6 Files Affected:
- (modified) flang/test/Fir/box.fir (+2-2)
- (modified) flang/test/Fir/rebox-global.fir (+1-1)
- (modified) flang/test/Fir/type-descriptor.fir (+2-2)
- (modified) mlir/lib/Target/LLVMIR/ModuleTranslation.cpp (+7-2)
- (modified) mlir/test/Target/LLVMIR/llvmir.mlir (+1-1)
- (modified) mlir/test/Target/LLVMIR/omptarget-declare-target-llvm-host.mlir (+1-1)
``````````diff
diff --git a/flang/test/Fir/box.fir b/flang/test/Fir/box.fir
index d4a51578883674..ef6be4d9b513e9 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<!fir.heap<i32>> {
%c0 = arith.constant 0 : index
%0 = fir.convert %c0 : (index) -> !fir.heap<i32>
@@ -9,7 +9,7 @@ fir.global internal @globalx : !fir.box<!fir.heap<i32>> {
fir.has_value %1 : !fir.box<!fir.heap<i32>>
}
-// 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<!fir.heap<!fir.array<?xf32>>> {
%c0 = arith.constant 0 : index
%0 = fir.convert %c0 : (index) -> !fir.heap<!fir.array<?xf32>>
diff --git a/flang/test/Fir/rebox-global.fir b/flang/test/Fir/rebox-global.fir
index 509487ab61d53b..873adc123322c2 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.ptr<!fir.char<4,10>>> {
fir.has_value %2 : !fir.box<!fir.ptr<!fir.char<4,10>>>
}
// 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 3b58a2f68251a7..ab48caeb4d199d 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.heap<!sometype>> {
fir.has_value %1 : !fir.box<!fir.heap<!sometype>>
}
// 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<!fir.ptr<!fir.array<?x?xf32>>>}>
@@ -25,5 +25,5 @@ fir.global internal @_QFfooEx2 : !fir.box<!fir.heap<!some_pdt_type>> {
fir.has_value %1 : !fir.box<!fir.heap<!some_pdt_type>>
}
// 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 ed61cb255be8fa..caea4ca44d1437 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<LLVM::GlobalOp>()) {
if (Block *initializer = op.getInitializerBlock()) {
- llvm::IRBuilder<> builder(llvmModule->getContext());
+ llvm::IRBuilder<llvm::TargetFolder> 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<LLVM::AliasOp>()) {
Block &initializer = op.getInitializerBlock();
- llvm::IRBuilder<> builder(llvmModule->getContext());
+ llvm::IRBuilder<llvm::TargetFolder> 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 52aa69f4c481f9..c0b3a9cb430220 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 0700c378b0056b..92c85738dbc72d 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"
``````````
</details>
https://github.com/llvm/llvm-project/pull/126745
More information about the Mlir-commits
mailing list