[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:10:00 PDT 2023


llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-flang-fir-hlfir

<details>
<summary>Changes</summary>

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.

---
Full diff: https://github.com/llvm/llvm-project/pull/67693.diff


5 Files Affected:

- (modified) flang/docs/Extensions.md (+1-1) 
- (modified) flang/lib/Lower/ConvertVariable.cpp (+9-14) 
- (modified) flang/lib/Optimizer/CodeGen/CodeGen.cpp (+1-14) 
- (modified) flang/test/Fir/convert-to-llvm-invalid.fir (-11) 
- (modified) flang/test/Fir/convert-to-llvm.fir (+24-10) 


``````````diff
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

``````````

</details>


https://github.com/llvm/llvm-project/pull/67693


More information about the flang-commits mailing list