[flang-commits] [flang] [flang] zero initialized all saved values without initial values (PR #67693)
via flang-commits
flang-commits at lists.llvm.org
Thu Sep 28 08:08:25 PDT 2023
https://github.com/jeanPerier created https://github.com/llvm/llvm-project/pull/67693
This is not standard but is vastly expected by existing code.
This was implemented by https://reviews.llvm.org/D149877 for simple scalars, but MLIR lacked a generic way to deal with aggregate types (arrays and derived type).
Support was recently added in https://github.com/llvm/llvm-project/pull/65508. Leverage it to zero initialize all types.
>From 65e18a823f9d6075fbc7dd098dbcda06a93fcc4b Mon Sep 17 00:00:00 2001
From: Jean Perier <jperier at nvidia.com>
Date: Thu, 28 Sep 2023 07:57:46 -0700
Subject: [PATCH] [flang] zero initialized all saved values without initial
values
This is not standard but is vastly expected by existing code.
This was implemented by https://reviews.llvm.org/D149877 for simple
scalars, but MLIR lacked a generic way to deal with aggregate types
(arrays and derived type).
Support was recently added in https://github.com/llvm/llvm-project/pull/65508.
Leverage it to zero initialize all types.
---
flang/docs/Extensions.md | 2 +-
flang/lib/Lower/ConvertVariable.cpp | 23 ++++++---------
flang/lib/Optimizer/CodeGen/CodeGen.cpp | 15 +---------
flang/test/Fir/convert-to-llvm-invalid.fir | 11 -------
flang/test/Fir/convert-to-llvm.fir | 34 +++++++++++++++-------
5 files changed, 35 insertions(+), 50 deletions(-)
diff --git a/flang/docs/Extensions.md b/flang/docs/Extensions.md
index bacb2dd7996b930..c1591eb49732b7f 100644
--- a/flang/docs/Extensions.md
+++ b/flang/docs/Extensions.md
@@ -100,7 +100,7 @@ end
* `<>` as synonym for `.NE.` and `/=`
* `$` and `@` as legal characters in names
* Initialization in type declaration statements using `/values/`
-* Saved integer, logical and real scalars are zero initialized.
+* Saved variables without explicit or default initializers are zero initialized.
* Kind specification with `*`, e.g. `REAL*4`
* `DOUBLE COMPLEX` as a synonym for `COMPLEX(KIND(0.D0))` --
but not when spelled `TYPE(DOUBLECOMPLEX)`.
diff --git a/flang/lib/Lower/ConvertVariable.cpp b/flang/lib/Lower/ConvertVariable.cpp
index 58d754668000ef3..87b570d1ac4b7e2 100644
--- a/flang/lib/Lower/ConvertVariable.cpp
+++ b/flang/lib/Lower/ConvertVariable.cpp
@@ -504,25 +504,20 @@ static fir::GlobalOp defineGlobal(Fortran::lower::AbstractConverter &converter,
} else {
TODO(loc, "global"); // Procedure pointer or something else
}
- // Creates zero or undefined initializer for globals without initializers
- // Zero initializer is used for "simple types" (integer, real and logical),
- // undefined is used for types aside from those types.
+ // Creates zero initializer for globals without initializers, this is a common
+ // and expected behavior (although not required by the standard)
if (!globalIsInitialized(global)) {
- // TODO: Is it really required to add the undef init if the Public
- // visibility is set ? We need to make sure the global is not optimized out
- // by LLVM if unused in the current compilation unit, but at least for
- // BIND(C) variables, an initial value may be given in another compilation
- // unit (on the C side), and setting an undef init here creates linkage
- // conflicts.
+ // TODO: For BIND(C) variables, an initial value may be given in another
+ // compilation unit (on the C side), and setting an zero init here creates
+ // linkage conflicts. See if there is a way to get it zero initialized if
+ // not initialized elsewhere. MLIR also used to drop globals without
+ // initializers that are not used in the file, but this may not be true
+ // anymore.
if (sym.attrs().test(Fortran::semantics::Attr::BIND_C))
TODO(loc, "BIND(C) module variable linkage");
Fortran::lower::createGlobalInitialization(
builder, global, [&](fir::FirOpBuilder &builder) {
- mlir::Value initValue;
- if (symTy.isa<mlir::IntegerType, mlir::FloatType, fir::LogicalType>())
- initValue = builder.create<fir::ZeroOp>(loc, symTy);
- else
- initValue = builder.create<fir::UndefOp>(loc, symTy);
+ mlir::Value initValue = builder.create<fir::ZeroOp>(loc, symTy);
builder.create<fir::HasValueOp>(loc, initValue);
});
}
diff --git a/flang/lib/Optimizer/CodeGen/CodeGen.cpp b/flang/lib/Optimizer/CodeGen/CodeGen.cpp
index 9d92086f8d70908..d1b7f3de93b4647 100644
--- a/flang/lib/Optimizer/CodeGen/CodeGen.cpp
+++ b/flang/lib/Optimizer/CodeGen/CodeGen.cpp
@@ -3406,20 +3406,7 @@ struct ZeroOpConversion : public FIROpConversion<fir::ZeroOp> {
matchAndRewrite(fir::ZeroOp zero, OpAdaptor,
mlir::ConversionPatternRewriter &rewriter) const override {
mlir::Type ty = convertType(zero.getType());
- if (ty.isa<mlir::LLVM::LLVMPointerType>()) {
- rewriter.replaceOpWithNewOp<mlir::LLVM::ZeroOp>(zero, ty);
- } else if (ty.isa<mlir::IntegerType>()) {
- rewriter.replaceOpWithNewOp<mlir::LLVM::ConstantOp>(
- zero, ty, mlir::IntegerAttr::get(ty, 0));
- } else if (mlir::LLVM::isCompatibleFloatingPointType(ty)) {
- rewriter.replaceOpWithNewOp<mlir::LLVM::ConstantOp>(
- zero, ty, mlir::FloatAttr::get(zero.getType(), 0.0));
- } else {
- // TODO: create ConstantAggregateZero for FIR aggregate/array types.
- return rewriter.notifyMatchFailure(
- zero,
- "conversion of fir.zero with aggregate type not implemented yet");
- }
+ rewriter.replaceOpWithNewOp<mlir::LLVM::ZeroOp>(zero, ty);
return mlir::success();
}
};
diff --git a/flang/test/Fir/convert-to-llvm-invalid.fir b/flang/test/Fir/convert-to-llvm-invalid.fir
index bdc2525876ac280..fcd042f83bd03c0 100644
--- a/flang/test/Fir/convert-to-llvm-invalid.fir
+++ b/flang/test/Fir/convert-to-llvm-invalid.fir
@@ -2,17 +2,6 @@
// RUN: fir-opt --split-input-file --fir-to-llvm-ir="target=x86_64-unknown-linux-gnu" --verify-diagnostics %s
-// Test `fir.zero` conversion failure with aggregate type.
-// Not implemented yet.
-
-func.func @zero_aggregate() {
- // expected-error at +1{{failed to legalize operation 'fir.zero_bits'}}
- %a = fir.zero_bits !fir.array<10xf32>
- return
-}
-
-// -----
-
// Verify that `fir.dt_entry` requires a parent op
// expected-error at +1{{'fir.dt_entry' op expects parent op 'fir.dispatch_table'}}
diff --git a/flang/test/Fir/convert-to-llvm.fir b/flang/test/Fir/convert-to-llvm.fir
index e39f13ac98268e8..30695f115a60bd8 100644
--- a/flang/test/Fir/convert-to-llvm.fir
+++ b/flang/test/Fir/convert-to-llvm.fir
@@ -160,10 +160,10 @@ func.func @zero_test_integer() {
return
}
-// CHECK: %{{.*}} = llvm.mlir.constant(0 : i8) : i8
-// CHECK: %{{.*}} = llvm.mlir.constant(0 : i16) : i16
-// CHECK: %{{.*}} = llvm.mlir.constant(0 : i32) : i32
-// CHECK: %{{.*}} = llvm.mlir.constant(0 : i64) : i64
+// CHECK: %{{.*}} = llvm.mlir.zero : i8
+// CHECK: %{{.*}} = llvm.mlir.zero : i16
+// CHECK: %{{.*}} = llvm.mlir.zero : i32
+// CHECK: %{{.*}} = llvm.mlir.zero : i64
// CHECK-NOT: fir.zero_bits
// -----
@@ -180,16 +180,30 @@ func.func @zero_test_float() {
return
}
-// CHECK: %{{.*}} = llvm.mlir.constant(0.000000e+00 : f16) : f16
-// CHECK: %{{.*}} = llvm.mlir.constant(0.000000e+00 : bf16) : bf16
-// CHECK: %{{.*}} = llvm.mlir.constant(0.000000e+00 : f32) : f32
-// CHECK: %{{.*}} = llvm.mlir.constant(0.000000e+00 : f64) : f64
-// CHECK: %{{.*}} = llvm.mlir.constant(0.000000e+00 : f80) : f80
-// CHECK: %{{.*}} = llvm.mlir.constant(0.000000e+00 : f128) : f128
+// CHECK: %{{.*}} = llvm.mlir.zero : f16
+// CHECK: %{{.*}} = llvm.mlir.zero : bf16
+// CHECK: %{{.*}} = llvm.mlir.zero : f32
+// CHECK: %{{.*}} = llvm.mlir.zero : f64
+// CHECK: %{{.*}} = llvm.mlir.zero : f80
+// CHECK: %{{.*}} = llvm.mlir.zero : f128
// CHECK-NOT: fir.zero_bits
// -----
+// Test fir.zero_bits with aggregate types.
+
+func.func @zero_aggregate() {
+ %a = fir.zero_bits !fir.array<10xf32>
+ %b = fir.zero_bits !fir.type<a{i:i32,j:f32}>
+ return
+}
+// CHECK: %{{.*}} = llvm.mlir.zero : !llvm.array<10 x f32>
+// CHECK: %{{.*}} = llvm.mlir.zero : !llvm.struct<"a", (i32, f32)>
+// CHECK-NOT: fir.zero_bits
+
+// -----
+
+
// Verify that fir.allocmem is transformed to a call to malloc
// and that fir.freemem is transformed to a call to free
// Single item case
More information about the flang-commits
mailing list