[MLIR][LLVMIR] Use TargetFolder when creating globals #126745
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
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 (llvm#96047).
llvmbot
added
mlir:llvm
mlir
flang
|
@llvm/pr-subscribers-mlir-llvm Author: Nikita Popov (nikic) ChangesThe 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 (#96047). Full diff: https://github.com/llvm/llvm-project/pull/126745.diff 6 Files Affected:
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"
|
|
@llvm/pr-subscribers-mlir Author: Nikita Popov (nikic) ChangesThe 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 (#96047). Full diff: https://github.com/llvm/llvm-project/pull/126745.diff 6 Files Affected:
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<!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 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.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 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.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 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<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 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"
|
gysit
approved these changes
Feb 11, 2025
There was a problem hiding this comment.
LGTM
Thanks for the fix!
Is this correctness issue limited to globals, or could this also be an issue when translating the operations within function bodies? Does the llvm builder always fold or does the constant folding happen once the expression is casted to a constant expression (the latter is only done for globals/aliases I believe)?
If constant folding is not possible, IRBuilder will create instructions, which is fine for function bodies but not for globals. Using TargetFolder for function bodies still makes sense though, to avoid generating unnecessary instructions for cases that can be constant folded. I'll probably try doing that in a followup.
The IRBuilder always does constant folding on the fly (well, unless you disable it entirely using NoFolder). |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/203/builds/1750 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/205/builds/550 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/204/builds/560 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/89/builds/16430 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/138/builds/10201 Here is the relevant piece of the build log for the reference |
nikic
added a commit
that referenced
this pull request
Feb 12, 2025
After #126745, we should also depend on the Analysis component.
nikic
added a commit
to nikic/llvm-project
that referenced
this pull request
Feb 12, 2025
This is a followup to llvm#126745, generalizing it to always use TargetFolder, including inside function bodies. This avoids generating constant expressions that can be folded away, and reduces changes when constant expressions get removed over time.
nikic
added a commit
that referenced
this pull request
Feb 13, 2025
This is a followup to #126745, generalizing it to always use TargetFolder, including inside function bodies. This avoids generating non-canonical constant expressions that can be folded away.
github-actions bot
pushed a commit
to arm/arm-toolchain
that referenced
this pull request
Feb 13, 2025
This is a followup to llvm/llvm-project#126745, generalizing it to always use TargetFolder, including inside function bodies. This avoids generating non-canonical constant expressions that can be folded away.
flovent
pushed a commit
to flovent/llvm-project
that referenced
this pull request
Feb 13, 2025
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 (llvm#96047).
flovent
pushed a commit
to flovent/llvm-project
that referenced
this pull request
Feb 13, 2025
After llvm#126745, we should also depend on the Analysis component.
flovent
pushed a commit
to flovent/llvm-project
that referenced
this pull request
Feb 13, 2025
This is a followup to llvm#126745, generalizing it to always use TargetFolder, including inside function bodies. This avoids generating non-canonical constant expressions that can be folded away.
joaosaffran
pushed a commit
to joaosaffran/llvm-project
that referenced
this pull request
Feb 14, 2025
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 (llvm#96047).
joaosaffran
pushed a commit
to joaosaffran/llvm-project
that referenced
this pull request
Feb 14, 2025
After llvm#126745, we should also depend on the Analysis component.
joaosaffran
pushed a commit
to joaosaffran/llvm-project
that referenced
this pull request
Feb 14, 2025
This is a followup to llvm#126745, generalizing it to always use TargetFolder, including inside function bodies. This avoids generating non-canonical constant expressions that can be folded away.
sivan-shani
pushed a commit
to sivan-shani/llvm-project
that referenced
this pull request
Feb 24, 2025
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 (llvm#96047).
sivan-shani
pushed a commit
to sivan-shani/llvm-project
that referenced
this pull request
Feb 24, 2025
After llvm#126745, we should also depend on the Analysis component.
sivan-shani
pushed a commit
to sivan-shani/llvm-project
that referenced
this pull request
Feb 24, 2025
This is a followup to llvm#126745, generalizing it to always use TargetFolder, including inside function bodies. This avoids generating non-canonical constant expressions that can be folded away.
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.
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 (#96047).