[flang] zero initialized all saved values without initial values #67693
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 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 llvm#65508. Leverage it to zero initialize all types.
llvmbot
added
flang
|
@llvm/pr-subscribers-flang-codegen @llvm/pr-subscribers-flang-fir-hlfir ChangesThis 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 #65508. Leverage it to zero initialize all types. Full diff: https://github.com/llvm/llvm-project/pull/67693.diff 5 Files Affected:
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@+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@+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
|
legrosbuffle
pushed a commit
to legrosbuffle/llvm-project
that referenced
this pull request
Sep 29, 2023
…m#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 llvm#65508. Leverage it to zero initialize all types.
jeanPerier
added a commit
to jeanPerier/llvm-project
that referenced
this pull request
Sep 29, 2023
Follow up up of llvm#67693 - Zero initialize uninitialized components of saved derived type entity with a default initial value. - Zero initialize uninitialized storage of common blocks with a member with an initial value. - Zero initialized uninitialized saved equivalence This removes all the cases where fir.global are created with an initial value that results in an undef in LLVM for part of the global, leading in surprising LLVM optimizations at -O2 for Fortran folks that expects there saved variables to be zero initialized if there is no explicit or default initial value.
jeanPerier
added a commit
that referenced
this pull request
Oct 2, 2023
#67777) Follow up up of #67693 - Zero initialize uninitialized components of saved derived type entity with a default initial value. - Zero initialize uninitialized storage of common blocks with a member with an initial value. - Zero initialized uninitialized saved equivalence This removes all the cases where fir.global are created with an initial value that results in an undef in LLVM for part of the global, leading in surprising LLVM optimizations at -O2 for Fortran folks that expects there saved variables to be zero initialized if there is no explicit or default initial value.
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 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 #65508. Leverage it to zero initialize all types.