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

Valentin Clement バレンタイン クレメン via flang-commits flang-commits at lists.llvm.org
Mon Jun 3 13:35:31 PDT 2024


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

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. 

>From 583b5f9c5f81d856abf83136049023b5c52c1542 Mon Sep 17 00:00:00 2001
From: Valentin Clement <clementval at gmail.com>
Date: Thu, 30 May 2024 21:43:52 -0700
Subject: [PATCH] [flang] Carry over alignment computed by frontend for COMMON

---
 flang/include/flang/Optimizer/Dialect/FIROps.td           | 3 ++-
 flang/lib/Lower/ConvertVariable.cpp                       | 8 +++++++-
 flang/lib/Optimizer/CodeGen/CodeGen.cpp                   | 3 +++
 flang/test/Lower/OpenMP/declare-target-data.f90           | 8 ++++----
 flang/test/Lower/OpenMP/lastprivate-commonblock.f90       | 2 +-
 flang/test/Lower/OpenMP/threadprivate-commonblock-use.f90 | 2 +-
 flang/test/Lower/OpenMP/threadprivate-commonblock.f90     | 2 +-
 flang/test/Lower/OpenMP/threadprivate-use-association.f90 | 2 +-
 flang/test/Lower/common-block-2.f90                       | 4 ++--
 flang/test/Lower/common-block.f90                         | 7 +++++++
 flang/test/Lower/module_definition.f90                    | 6 +++---
 flang/test/Lower/module_use.f90                           | 6 +++---
 flang/test/Lower/pointer-initial-target-2.f90             | 8 ++++----
 13 files changed, 39 insertions(+), 22 deletions(-)

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



More information about the flang-commits mailing list