[flang-commits] [flang] [flang] Carry over alignment computed by frontend for COMMON (PR #94280)

via flang-commits flang-commits at lists.llvm.org
Mon Jun 3 13:36:02 PDT 2024 

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-flang-openmp

@llvm/pr-subscribers-flang-codegen

Author: Valentin Clement (バレンタイン クレメン) (clementval)

<details>
<summary>Changes</summary>

The frontend computes the necessary alignment for COMMON blocks but this information is never carried over to the code generation and can lead to segfault for COMMON block that requires a non default alignment. 

This patch add an optional attribute on fir.global and carries over the information. 

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


13 Files Affected:

- (modified) flang/include/flang/Optimizer/Dialect/FIROps.td (+2-1) 
- (modified) flang/lib/Lower/ConvertVariable.cpp (+7-1) 
- (modified) flang/lib/Optimizer/CodeGen/CodeGen.cpp (+3) 
- (modified) flang/test/Lower/OpenMP/declare-target-data.f90 (+4-4) 
- (modified) flang/test/Lower/OpenMP/lastprivate-commonblock.f90 (+1-1) 
- (modified) flang/test/Lower/OpenMP/threadprivate-commonblock-use.f90 (+1-1) 
- (modified) flang/test/Lower/OpenMP/threadprivate-commonblock.f90 (+1-1) 
- (modified) flang/test/Lower/OpenMP/threadprivate-use-association.f90 (+1-1) 
- (modified) flang/test/Lower/common-block-2.f90 (+2-2) 
- (modified) flang/test/Lower/common-block.f90 (+7) 
- (modified) flang/test/Lower/module_definition.f90 (+3-3) 
- (modified) flang/test/Lower/module_use.f90 (+3-3) 
- (modified) flang/test/Lower/pointer-initial-target-2.f90 (+4-4) 


``````````diff
diff --git a/flang/include/flang/Optimizer/Dialect/FIROps.td b/flang/include/flang/Optimizer/Dialect/FIROps.td
index 9ea70d5057c9a..d46e997c7d8fe 100644
--- a/flang/include/flang/Optimizer/Dialect/FIROps.td
+++ b/flang/include/flang/Optimizer/Dialect/FIROps.td
@@ -2800,7 +2800,8 @@ def fir_GlobalOp : fir_Op<"global", [IsolatedFromAbove, Symbol]> {
     OptionalAttr<UnitAttr>:$constant,
     OptionalAttr<UnitAttr>:$target,
     OptionalAttr<StrAttr>:$linkName,
-    OptionalAttr<cuf_DataAttributeAttr>:$data_attr
+    OptionalAttr<cuf_DataAttributeAttr>:$data_attr,
+    OptionalAttr<I64Attr>:$alignment
   );
 
   let regions = (region AtMostRegion<1>:$region);
diff --git a/flang/lib/Lower/ConvertVariable.cpp b/flang/lib/Lower/ConvertVariable.cpp
index c15d6b682bdb6..22b3f763a2034 100644
--- a/flang/lib/Lower/ConvertVariable.cpp
+++ b/flang/lib/Lower/ConvertVariable.cpp
@@ -1307,7 +1307,10 @@ declareCommonBlock(Fortran::lower::AbstractConverter &converter,
     auto vecTy = mlir::VectorType::get(sz, i8Ty);
     mlir::Attribute zero = builder.getIntegerAttr(i8Ty, 0);
     auto init = mlir::DenseElementsAttr::get(vecTy, llvm::ArrayRef(zero));
-    builder.createGlobal(loc, commonTy, commonName, linkage, init);
+    global = builder.createGlobal(loc, commonTy, commonName, linkage, init);
+    if (const auto *details =
+            common.detailsIf<Fortran::semantics::CommonBlockDetails>())
+      global.setAlignment(details->alignment());
     // No need to add any initial value later.
     return std::nullopt;
   }
@@ -1320,6 +1323,9 @@ declareCommonBlock(Fortran::lower::AbstractConverter &converter,
       getTypeOfCommonWithInit(converter, cmnBlkMems, commonSize);
   // Create the global object, the initial value will be added later.
   global = builder.createGlobal(loc, commonTy, commonName);
+  if (const auto *details =
+          common.detailsIf<Fortran::semantics::CommonBlockDetails>())
+    global.setAlignment(details->alignment());
   return std::make_tuple(global, std::move(cmnBlkMems), loc);
 }
 
diff --git a/flang/lib/Optimizer/CodeGen/CodeGen.cpp b/flang/lib/Optimizer/CodeGen/CodeGen.cpp
index 5d3adc3dfbf47..9f21c6b0cf097 100644
--- a/flang/lib/Optimizer/CodeGen/CodeGen.cpp
+++ b/flang/lib/Optimizer/CodeGen/CodeGen.cpp
@@ -2742,6 +2742,9 @@ struct GlobalOpConversion : public fir::FIROpConversion<fir::GlobalOp> {
         loc, tyAttr, isConst, linkage, global.getSymName(), initAttr, 0, 0,
         false, false, comdat, attrs, dbgExpr);
 
+    if (global.getAlignment() && *global.getAlignment() > 0)
+      g.setAlignment(*global.getAlignment());
+
     auto module = global->getParentOfType<mlir::ModuleOp>();
     // Add comdat if necessary
     if (fir::getTargetTriple(module).supportsCOMDAT() &&
diff --git a/flang/test/Lower/OpenMP/declare-target-data.f90 b/flang/test/Lower/OpenMP/declare-target-data.f90
index 4568810d8d07d..d86f74d18b6df 100644
--- a/flang/test/Lower/OpenMP/declare-target-data.f90
+++ b/flang/test/Lower/OpenMP/declare-target-data.f90
@@ -62,25 +62,25 @@ module test_0
 end module test_0
 
 PROGRAM commons
-    !CHECK-DAG: fir.global @numbers_ {omp.declare_target = #omp.declaretarget<device_type = (any), capture_clause = (to)>} : tuple<f32, f32> {
+    !CHECK-DAG: fir.global @numbers_ {alignment = 4 : i64, omp.declare_target = #omp.declaretarget<device_type = (any), capture_clause = (to)>} : tuple<f32, f32> {
     REAL :: one = 1
     REAL :: two = 2
     COMMON /numbers/ one, two
     !$omp declare target(/numbers/)
     
-    !CHECK-DAG: fir.global @numbers_link_ {omp.declare_target = #omp.declaretarget<device_type = (any), capture_clause = (link)>} : tuple<f32, f32> {
+    !CHECK-DAG: fir.global @numbers_link_ {alignment = 4 : i64, omp.declare_target = #omp.declaretarget<device_type = (any), capture_clause = (link)>} : tuple<f32, f32> {
     REAL :: one_link = 1
     REAL :: two_link = 2
     COMMON /numbers_link/ one_link, two_link
     !$omp declare target link(/numbers_link/)
 
-    !CHECK-DAG: fir.global @numbers_to_ {omp.declare_target = #omp.declaretarget<device_type = (any), capture_clause = (to)>} : tuple<f32, f32> {
+    !CHECK-DAG: fir.global @numbers_to_ {alignment = 4 : i64, omp.declare_target = #omp.declaretarget<device_type = (any), capture_clause = (to)>} : tuple<f32, f32> {
     REAL :: one_to = 1
     REAL :: two_to = 2
     COMMON /numbers_to/ one_to, two_to
     !$omp declare target to(/numbers_to/)
 
-    !CHECK-DAG: fir.global @numbers_enter_ {omp.declare_target = #omp.declaretarget<device_type = (any), capture_clause = (enter)>} : tuple<f32, f32> {
+    !CHECK-DAG: fir.global @numbers_enter_ {alignment = 4 : i64, omp.declare_target = #omp.declaretarget<device_type = (any), capture_clause = (enter)>} : tuple<f32, f32> {
     REAL :: one_enter = 1
     REAL :: two_enter = 2
     COMMON /numbers_enter/ one_enter, two_enter
diff --git a/flang/test/Lower/OpenMP/lastprivate-commonblock.f90 b/flang/test/Lower/OpenMP/lastprivate-commonblock.f90
index 78adf09c6fe34..f823bf6c56ae3 100644
--- a/flang/test/Lower/OpenMP/lastprivate-commonblock.f90
+++ b/flang/test/Lower/OpenMP/lastprivate-commonblock.f90
@@ -1,6 +1,6 @@
 ! RUN: %flang_fc1 -emit-hlfir -fopenmp -o - %s 2>&1 | FileCheck %s
 
-!CHECK: fir.global common @[[CB_C:.*]](dense<0> : vector<8xi8>) : !fir.array<8xi8>
+!CHECK: fir.global common @[[CB_C:.*]](dense<0> : vector<8xi8>) {alignment = 4 : i64} : !fir.array<8xi8>
 !CHECK-LABEL: func.func @_QPlastprivate_common
 !CHECK:      %[[CB_C_REF:.*]] = fir.address_of(@[[CB_C]]) : !fir.ref<!fir.array<8xi8>>
 !CHECK:      %[[CB_C_REF_CVT:.*]] = fir.convert %[[CB_C_REF]] : (!fir.ref<!fir.array<8xi8>>) -> !fir.ref<!fir.array<?xi8>>
diff --git a/flang/test/Lower/OpenMP/threadprivate-commonblock-use.f90 b/flang/test/Lower/OpenMP/threadprivate-commonblock-use.f90
index 71f1c7608a2c3..f02a7486caf5f 100644
--- a/flang/test/Lower/OpenMP/threadprivate-commonblock-use.f90
+++ b/flang/test/Lower/OpenMP/threadprivate-commonblock-use.f90
@@ -4,7 +4,7 @@
 
 !RUN: %flang_fc1 -emit-hlfir -fopenmp %s -o - | FileCheck %s
 
-!CHECK: fir.global common @cmn_(dense<0> : vector<4xi8>) : !fir.array<4xi8>
+!CHECK: fir.global common @cmn_(dense<0> : vector<4xi8>) {alignment = 4 : i64} : !fir.array<4xi8>
 module m0
   common /cmn/ k1
   !$omp threadprivate(/cmn/)
diff --git a/flang/test/Lower/OpenMP/threadprivate-commonblock.f90 b/flang/test/Lower/OpenMP/threadprivate-commonblock.f90
index ddbe5bd524657..eb6abe4d2cacd 100644
--- a/flang/test/Lower/OpenMP/threadprivate-commonblock.f90
+++ b/flang/test/Lower/OpenMP/threadprivate-commonblock.f90
@@ -12,7 +12,7 @@ module test
 
   !$omp threadprivate(/blk/)
 
-!CHECK: fir.global common @blk_(dense<0> : vector<103xi8>) : !fir.array<103xi8>
+!CHECK: fir.global common @blk_(dense<0> : vector<103xi8>) {alignment = 8 : i64} : !fir.array<103xi8>
 
 contains
   subroutine sub()
diff --git a/flang/test/Lower/OpenMP/threadprivate-use-association.f90 b/flang/test/Lower/OpenMP/threadprivate-use-association.f90
index d0d461547db2f..9dcb2ab3ff9ff 100644
--- a/flang/test/Lower/OpenMP/threadprivate-use-association.f90
+++ b/flang/test/Lower/OpenMP/threadprivate-use-association.f90
@@ -3,7 +3,7 @@
 
 !RUN: %flang_fc1 -emit-hlfir -fopenmp %s -o - | FileCheck %s
 
-!CHECK-DAG: fir.global common @blk_(dense<0> : vector<24xi8>) : !fir.array<24xi8>
+!CHECK-DAG: fir.global common @blk_(dense<0> : vector<24xi8>) {alignment = 4 : i64} : !fir.array<24xi8>
 !CHECK-DAG: fir.global @_QMtestEy : f32 {
 
 module test
diff --git a/flang/test/Lower/common-block-2.f90 b/flang/test/Lower/common-block-2.f90
index 4b12860a48ffa..635a1597b1033 100644
--- a/flang/test/Lower/common-block-2.f90
+++ b/flang/test/Lower/common-block-2.f90
@@ -5,12 +5,12 @@
 ! - A blank common that is initialized
 ! - A common block that is initialized outside of a BLOCK DATA.
 
-! CHECK-LABEL: fir.global @__BLNK__ : tuple<i32, !fir.array<8xi8>> {
+! CHECK-LABEL: fir.global @__BLNK__ {alignment = 4 : i64} : tuple<i32, !fir.array<8xi8>> {
 ! CHECK:  %[[undef:.*]] = fir.zero_bits tuple<i32, !fir.array<8xi8>>
 ! CHECK:  %[[init:.*]] = fir.insert_value %[[undef]], %c42{{.*}}, [0 : index] : (tuple<i32, !fir.array<8xi8>>, i32) -> tuple<i32, !fir.array<8xi8>>
 ! CHECK:  fir.has_value %[[init]] : tuple<i32, !fir.array<8xi8>>
 
-! CHECK-LABEL: fir.global @a_ : tuple<i32, !fir.array<8xi8>> {
+! CHECK-LABEL: fir.global @a_ {alignment = 4 : i64} : tuple<i32, !fir.array<8xi8>> {
 ! CHECK:  %[[undef:.*]] = fir.zero_bits tuple<i32, !fir.array<8xi8>>
 ! CHECK:  %[[init:.*]] = fir.insert_value %[[undef]], %c42{{.*}}, [0 : index] : (tuple<i32, !fir.array<8xi8>>, i32) -> tuple<i32, !fir.array<8xi8>>
 ! CHECK:  fir.has_value %[[init]] : tuple<i32, !fir.array<8xi8>>
diff --git a/flang/test/Lower/common-block.f90 b/flang/test/Lower/common-block.f90
index 3934b71b75694..94e61b8bcc33d 100644
--- a/flang/test/Lower/common-block.f90
+++ b/flang/test/Lower/common-block.f90
@@ -2,6 +2,7 @@
 ! RUN: %flang -emit-llvm -S -mmlir -disable-external-name-interop %s -o - | FileCheck %s
 
 ! CHECK: @__BLNK__ = common global [8 x i8] zeroinitializer
+! CHECK: @co1_ = common global [16 x i8] zeroinitializer, align 16
 ! CHECK: @rien_ = common global [1 x i8] zeroinitializer
 ! CHECK: @with_empty_equiv_ = common global [8 x i8] zeroinitializer
 ! CHECK: @x_ = global { float, float } { float 1.0{{.*}}, float 2.0{{.*}} }
@@ -72,3 +73,9 @@ subroutine s6
   common /with_empty_equiv/ x, r1, y
   equivalence(r1, r2)
 end subroutine s6
+
+subroutine s7()
+  real(16) r16
+  common /co1/ r16
+end subroutine
+
diff --git a/flang/test/Lower/module_definition.f90 b/flang/test/Lower/module_definition.f90
index fd3b89db7d20c..0a05364ca473c 100644
--- a/flang/test/Lower/module_definition.f90
+++ b/flang/test/Lower/module_definition.f90
@@ -12,15 +12,15 @@ module modCommonNoInit1
   real :: x_named1
   common /named1/ x_named1
 end module
-! CHECK-LABEL: fir.global common @__BLNK__(dense<0> : vector<4xi8>) : !fir.array<4xi8>
-! CHECK-LABEL: fir.global common @named1_(dense<0> : vector<4xi8>) : !fir.array<4xi8>
+! CHECK-LABEL: fir.global common @__BLNK__(dense<0> : vector<4xi8>) {alignment = 4 : i64} : !fir.array<4xi8>
+! CHECK-LABEL: fir.global common @named1_(dense<0> : vector<4xi8>) {alignment = 4 : i64} : !fir.array<4xi8>
 
 ! Module defines variable in common block with initialization
 module modCommonInit1
   integer :: i_named2 = 42
   common /named2/ i_named2
 end module
-! CHECK-LABEL: fir.global @named2_ : tuple<i32> {
+! CHECK-LABEL: fir.global @named2_ {alignment = 4 : i64} : tuple<i32> {
   ! CHECK: %[[init:.*]] = fir.insert_value %{{.*}}, %c42{{.*}}, [0 : index] : (tuple<i32>, i32) -> tuple<i32>
   ! CHECK: fir.has_value %[[init]] : tuple<i32>
 
diff --git a/flang/test/Lower/module_use.f90 b/flang/test/Lower/module_use.f90
index 21458bb488430..ad43865470b68 100644
--- a/flang/test/Lower/module_use.f90
+++ b/flang/test/Lower/module_use.f90
@@ -5,9 +5,9 @@
 ! The modules are defined in module_definition.f90
 ! The first runs ensures the module file is generated.
 
-! CHECK: fir.global common @__BLNK__(dense<0> : vector<4xi8>) : !fir.array<4xi8>
-! CHECK-NEXT: fir.global common @named1_(dense<0> : vector<4xi8>) : !fir.array<4xi8>
-! CHECK-NEXT: fir.global common @named2_(dense<0> : vector<4xi8>) : !fir.array<4xi8>
+! CHECK: fir.global common @__BLNK__(dense<0> : vector<4xi8>) {alignment = 4 : i64} : !fir.array<4xi8>
+! CHECK-NEXT: fir.global common @named1_(dense<0> : vector<4xi8>) {alignment = 4 : i64} : !fir.array<4xi8>
+! CHECK-NEXT: fir.global common @named2_(dense<0> : vector<4xi8>) {alignment = 4 : i64} : !fir.array<4xi8>
 
 ! CHECK-LABEL: func @_QPm1use()
 real function m1use()
diff --git a/flang/test/Lower/pointer-initial-target-2.f90 b/flang/test/Lower/pointer-initial-target-2.f90
index 1129ddd03560d..99c7ede50504c 100644
--- a/flang/test/Lower/pointer-initial-target-2.f90
+++ b/flang/test/Lower/pointer-initial-target-2.f90
@@ -11,7 +11,7 @@
   real, save, target :: b
   common /a/ p
   data p /b/
-! CHECK-LABEL: fir.global @a_ : tuple<!fir.box<!fir.ptr<f32>>>
+! CHECK-LABEL: fir.global @a_ {alignment = 8 : i64} : tuple<!fir.box<!fir.ptr<f32>>>
   ! CHECK: %[[undef:.*]] = fir.zero_bits tuple<!fir.box<!fir.ptr<f32>>>
   ! CHECK: %[[b:.*]] = fir.address_of(@_QEb) : !fir.ref<f32>
   ! CHECK: %[[box:.*]] = fir.embox %[[b]] : (!fir.ref<f32>) -> !fir.box<f32>
@@ -29,9 +29,9 @@ block data tied
   real, pointer :: p2 => x1
   common /c1/ x1, p1
   common /c2/ x2, p2
-! CHECK-LABEL: fir.global @c1_ : tuple<f32, !fir.array<4xi8>, !fir.box<!fir.ptr<f32>>>
+! CHECK-LABEL: fir.global @c1_ {alignment = 8 : i64} : tuple<f32, !fir.array<4xi8>, !fir.box<!fir.ptr<f32>>>
   ! CHECK: fir.address_of(@c2_) : !fir.ref<tuple<f32, !fir.array<4xi8>, !fir.box<!fir.ptr<f32>>>>
-! CHECK-LABEL: fir.global @c2_ : tuple<f32, !fir.array<4xi8>, !fir.box<!fir.ptr<f32>>>
+! CHECK-LABEL: fir.global @c2_ {alignment = 8 : i64} : tuple<f32, !fir.array<4xi8>, !fir.box<!fir.ptr<f32>>>
   ! CHECK: fir.address_of(@c1_) : !fir.ref<tuple<f32, !fir.array<4xi8>, !fir.box<!fir.ptr<f32>>>>
 end block data
 
@@ -40,7 +40,7 @@ block data bdsnake
   integer, target :: b = 42
   integer, pointer :: p => b
   common /snake/ p, b
-! CHECK-LABEL: fir.global @snake_ : tuple<!fir.box<!fir.ptr<i32>>, i32>
+! CHECK-LABEL: fir.global @snake_ {alignment = 8 : i64} : tuple<!fir.box<!fir.ptr<i32>>, i32>
   ! CHECK: %[[tuple0:.*]] = fir.zero_bits tuple<!fir.box<!fir.ptr<i32>>, i32>
   ! CHECK: %[[snakeAddr:.*]] = fir.address_of(@snake_) : !fir.ref<tuple<!fir.box<!fir.ptr<i32>>, i32>>
   ! CHECK: %[[byteView:.*]] = fir.convert %[[snakeAddr:.*]] : (!fir.ref<tuple<!fir.box<!fir.ptr<i32>>, i32>>) -> !fir.ref<!fir.array<?xi8>>

``````````

</details>


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

More information about the flang-commits mailing list