From 8cb0c3bb2173890e5f6cf2082fe65682b15888ba Mon Sep 17 00:00:00 2001 From: David Truby Date: Mon, 26 Jun 2023 15:03:17 +0100 Subject: [PATCH] [flang] Add COMDAT to global variables where needed On platforms which support COMDAT sections we should use them when linkonce or linkonce_odr linkage is requested. This is required on Windows (PE/COFF) and provides better behaviour than weak symbols on ELF-based platforms. This patch also reverts string literals to use linkonce instead of internal linkage now that comdats are supported. Differential Revision: https://reviews.llvm.org/D153768 --- flang/lib/Optimizer/Builder/FIRBuilder.cpp | 4 +-- flang/lib/Optimizer/CodeGen/CodeGen.cpp | 29 ++++++++++++++++++++++ flang/test/Fir/convert-to-llvm.fir | 16 +++++++----- flang/test/Fir/tbaa.fir | 2 +- flang/test/Lower/allocatable-assignment.f90 | 2 +- flang/test/Lower/character-assignment.f90 | 2 +- flang/test/Lower/convert.f90 | 4 +-- flang/test/Lower/global-format-strings.f90 | 2 +- flang/test/Lower/io-statement-open-options.f90 | 2 +- flang/test/Lower/namelist.f90 | 6 ++--- flang/test/Lower/read-write-buffer.f90 | 2 +- .../unittests/Optimizer/Builder/FIRBuilderTest.cpp | 2 +- 12 files changed, 52 insertions(+), 21 deletions(-) diff --git a/flang/lib/Optimizer/Builder/FIRBuilder.cpp b/flang/lib/Optimizer/Builder/FIRBuilder.cpp index 5f98b10..fe23e13 100644 --- a/flang/lib/Optimizer/Builder/FIRBuilder.cpp +++ b/flang/lib/Optimizer/Builder/FIRBuilder.cpp @@ -999,9 +999,7 @@ fir::ExtendedValue fir::factory::createStringLiteral(fir::FirOpBuilder &builder, auto stringLitOp = builder.createStringLitOp(loc, str); builder.create(loc, stringLitOp); }, - builder.createInternalLinkage()); - // TODO: This can be changed to linkonce linkage once we have support for - // generating comdat sections + builder.createLinkOnceLinkage()); auto addr = builder.create(loc, global.resultType(), global.getSymbol()); auto len = builder.createIntegerConstant( diff --git a/flang/lib/Optimizer/CodeGen/CodeGen.cpp b/flang/lib/Optimizer/CodeGen/CodeGen.cpp index 2e207a9..0061d18 100644 --- a/flang/lib/Optimizer/CodeGen/CodeGen.cpp +++ b/flang/lib/Optimizer/CodeGen/CodeGen.cpp @@ -2856,6 +2856,14 @@ struct GlobalOpConversion : public FIROpConversion { auto g = rewriter.create( loc, tyAttr, isConst, linkage, global.getSymName(), initAttr); + auto module = global->getParentOfType(); + // Add comdat if necessary + if (fir::getTargetTriple(module).supportsCOMDAT() && + (linkage == mlir::LLVM::Linkage::Linkonce || + linkage == mlir::LLVM::Linkage::LinkonceODR)) { + addComdat(g, rewriter, module); + } + // Apply all non-Fir::GlobalOp attributes to the LLVM::GlobalOp, preserving // them; whilst taking care not to apply attributes that are lowered in // other ways. @@ -2931,6 +2939,27 @@ struct GlobalOpConversion : public FIROpConversion { } return mlir::LLVM::Linkage::External; } + +private: + static void addComdat(mlir::LLVM::GlobalOp &global, + mlir::ConversionPatternRewriter &rewriter, + mlir::ModuleOp &module) { + const char *comdatName = "__llvm_comdat"; + mlir::LLVM::ComdatOp comdatOp = + module.lookupSymbol(comdatName); + if (!comdatOp) { + comdatOp = + rewriter.create(module.getLoc(), comdatName); + } + mlir::OpBuilder::InsertionGuard guard(rewriter); + rewriter.setInsertionPointToEnd(&comdatOp.getBody().back()); + auto selectorOp = rewriter.create( + comdatOp.getLoc(), global.getSymName(), + mlir::LLVM::comdat::Comdat::Any); + global.setComdatAttr(mlir::SymbolRefAttr::get( + rewriter.getContext(), comdatName, + mlir::FlatSymbolRefAttr::get(selectorOp.getSymNameAttr()))); + } }; /// `fir.load` --> `llvm.load` diff --git a/flang/test/Fir/convert-to-llvm.fir b/flang/test/Fir/convert-to-llvm.fir index d1d6202..14cd58a 100644 --- a/flang/test/Fir/convert-to-llvm.fir +++ b/flang/test/Fir/convert-to-llvm.fir @@ -1,7 +1,9 @@ -// RUN: fir-opt --split-input-file --fir-to-llvm-ir="target=x86_64-unknown-linux-gnu" %s | FileCheck %s -// RUN: fir-opt --split-input-file --fir-to-llvm-ir="target=aarch64-unknown-linux-gnu" %s | FileCheck %s -// RUN: fir-opt --split-input-file --fir-to-llvm-ir="target=i386-unknown-linux-gnu" %s | FileCheck %s -// RUN: fir-opt --split-input-file --fir-to-llvm-ir="target=powerpc64le-unknown-linux-gn" %s | FileCheck %s +// RUN: fir-opt --split-input-file --fir-to-llvm-ir="target=x86_64-unknown-linux-gnu" %s | FileCheck %s --check-prefixes=CHECK,CHECK-COMDAT +// RUN: fir-opt --split-input-file --fir-to-llvm-ir="target=aarch64-unknown-linux-gnu" %s | FileCheck %s --check-prefixes=CHECK,CHECK-COMDAT +// RUN: fir-opt --split-input-file --fir-to-llvm-ir="target=i386-unknown-linux-gnu" %s | FileCheck %s --check-prefixes=CHECK,CHECK-COMDAT +// RUN: fir-opt --split-input-file --fir-to-llvm-ir="target=powerpc64le-unknown-linux-gn" %s | FileCheck %s --check-prefixes=CHECK,CHECK-COMDAT +// RUN: fir-opt --split-input-file --fir-to-llvm-ir="target=x86_64-pc-win32" %s | FileCheck %s --check-prefixes=CHECK,CHECK-COMDAT +// RUN: fir-opt --split-input-file --fir-to-llvm-ir="target=aarch64-apple-darwin" %s | FileCheck %s --check-prefixes=CHECK,CHECK-NO-COMDAT //============================================================================= // SUMMARY: Tests for FIR --> LLVM MLIR conversion independent of the target @@ -49,7 +51,8 @@ fir.global weak @w_i86 (86:i32) : i32 // ----- fir.global linkonce @w_i86 (86:i32) : i32 -// CHECK: llvm.mlir.global linkonce @w_i86(86 : i32) {addr_space = 0 : i32} : i32 +// CHECK-COMDAT: llvm.mlir.global linkonce @w_i86(86 : i32) comdat(@__llvm_comdat::@w_i86) {addr_space = 0 : i32} : i32 +// CHECK-NO-COMDAT: llvm.mlir.global linkonce @w_i86(86 : i32) {addr_space = 0 : i32} : i32 // ----- @@ -1678,7 +1681,8 @@ func.func @embox1(%arg0: !fir.ref>) { return } -// CHECK: llvm.mlir.global linkonce constant @_QMtest_dinitE.dt.tseq() {addr_space = 0 : i32} : i8 +// CHECK-COMDAT: llvm.mlir.global linkonce constant @_QMtest_dinitE.dt.tseq() comdat(@__llvm_comdat::@_QMtest_dinitE.dt.tseq) {addr_space = 0 : i32} : i8 +// CHECK-NO-COMDAT: llvm.mlir.global linkonce constant @_QMtest_dinitE.dt.tseq() {addr_space = 0 : i32} : i8 // CHECK-LABEL: llvm.func @embox1 // CHECK: %[[TYPE_CODE:.*]] = llvm.mlir.constant(42 : i32) : i32 // CHECK: %[[TYPE_CODE_I8:.*]] = llvm.trunc %[[TYPE_CODE]] : i32 to i8 diff --git a/flang/test/Fir/tbaa.fir b/flang/test/Fir/tbaa.fir index 66261e6..3528f61 100644 --- a/flang/test/Fir/tbaa.fir +++ b/flang/test/Fir/tbaa.fir @@ -195,7 +195,7 @@ module { // CHECK: llvm.func @_FortranAioOutputDescriptor(!llvm.ptr, !llvm.ptr>, i64, i32, i8, i8, i8, i8, ptr, array<1 x i64>)>>) -> i1 attributes {fir.io, fir.runtime, sym_visibility = "private"} // CHECK: llvm.func @_FortranAioEndIoStatement(!llvm.ptr) -> i32 attributes {fir.io, fir.runtime, sym_visibility = "private"} -// CHECK-LABEL: llvm.mlir.global linkonce constant @_QQcl.2E2F64756D6D792E66393000() {addr_space = 0 : i32} : !llvm.array<12 x i8> { +// CHECK-LABEL: llvm.mlir.global linkonce constant @_QQcl.2E2F64756D6D792E66393000() comdat(@__llvm_comdat::@_QQcl.2E2F64756D6D792E66393000) {addr_space = 0 : i32} : !llvm.array<12 x i8> { // CHECK: %[[VAL_0:.*]] = llvm.mlir.constant("./dummy.f90\00") : !llvm.array<12 x i8> // CHECK: llvm.return %[[VAL_0]] : !llvm.array<12 x i8> // CHECK: } diff --git a/flang/test/Lower/allocatable-assignment.f90 b/flang/test/Lower/allocatable-assignment.f90 index 60f6fac..5d2f281 100644 --- a/flang/test/Lower/allocatable-assignment.f90 +++ b/flang/test/Lower/allocatable-assignment.f90 @@ -1242,7 +1242,7 @@ subroutine test_both_sides_with_elemental_call(x) ! CHECK: } end subroutine -! CHECK: fir.global internal @[[error_message]] constant : !fir.char<1,76> { +! CHECK: fir.global linkonce @[[error_message]] constant : !fir.char<1,76> { ! CHECK: %[[msg:.*]] = fir.string_lit "array left hand side must be allocated when the right hand side is a scalar\00"(76) : !fir.char<1,76> ! CHECK: fir.has_value %[[msg:.*]] : !fir.char<1,76> ! CHECK: } diff --git a/flang/test/Lower/character-assignment.f90 b/flang/test/Lower/character-assignment.f90 index fad419e..7f1874d 100644 --- a/flang/test/Lower/character-assignment.f90 +++ b/flang/test/Lower/character-assignment.f90 @@ -102,7 +102,7 @@ end subroutine ! CHECK: return end subroutine -! CHECK-LABEL: fir.global internal @_QQcl.48656C6C6F20576F726C64 +! CHECK-LABEL: fir.global linkonce @_QQcl.48656C6C6F20576F726C64 ! CHECK: %[[lit:.*]] = fir.string_lit "Hello World"(11) : !fir.char<1,11> ! CHECK: fir.has_value %[[lit]] : !fir.char<1,11> ! CHECK: } diff --git a/flang/test/Lower/convert.f90 b/flang/test/Lower/convert.f90 index 5005019..1ab93dc 100755 --- a/flang/test/Lower/convert.f90 +++ b/flang/test/Lower/convert.f90 @@ -21,11 +21,11 @@ end ! ALL: %[[VAL_8:.*]] = fir.insert_value %[[VAL_4]], %[[VAL_7]], [0 : index, 1 : index] : (!fir.array<1xtuple, !fir.ref>>, !fir.ref) -> !fir.array<1xtuple, !fir.ref>> ! ALL: fir.has_value %[[VAL_8]] : !fir.array<1xtuple, !fir.ref>> -! ALL: fir.global internal @[[FC_STR]] constant : !fir.char<1,13> { +! ALL: fir.global linkonce @[[FC_STR]] constant : !fir.char<1,13> { ! ALL: %[[VAL_0:.*]] = fir.string_lit "FORT_CONVERT\00"(13) : !fir.char<1,13> ! ALL: fir.has_value %[[VAL_0]] : !fir.char<1,13> -! ALL: fir.global internal @[[OPT_STR]] constant : !fir.char<1,[[OPT_STR_LEN]]> { +! ALL: fir.global linkonce @[[OPT_STR]] constant : !fir.char<1,[[OPT_STR_LEN]]> { ! UNKNOWN: %[[VAL_0:.*]] = fir.string_lit "UNKNOWN\00"([[OPT_STR_LEN]]) : !fir.char<1,[[OPT_STR_LEN]]> ! NATIVE: %[[VAL_0:.*]] = fir.string_lit "NATIVE\00"([[OPT_STR_LEN]]) : !fir.char<1,[[OPT_STR_LEN]]> ! LITTLE_ENDIAN: %[[VAL_0:.*]] = fir.string_lit "LITTLE_ENDIAN\00"([[OPT_STR_LEN]]) : !fir.char<1,[[OPT_STR_LEN]]> diff --git a/flang/test/Lower/global-format-strings.f90 b/flang/test/Lower/global-format-strings.f90 index d9307ab..3112da3 100644 --- a/flang/test/Lower/global-format-strings.f90 +++ b/flang/test/Lower/global-format-strings.f90 @@ -8,7 +8,7 @@ program other ! CHECK: fir.address_of(@{{.*}}) : 1008 format('ok') end -! CHECK-LABEL: fir.global internal @_QQcl.28276F6B2729 constant +! CHECK-LABEL: fir.global linkonce @_QQcl.28276F6B2729 constant ! CHECK: %[[lit:.*]] = fir.string_lit "('ok')"(6) : !fir.char<1,6> ! CHECK: fir.has_value %[[lit]] : !fir.char<1,6> ! CHECK: } diff --git a/flang/test/Lower/io-statement-open-options.f90 b/flang/test/Lower/io-statement-open-options.f90 index 4d414e0..4348767 100755 --- a/flang/test/Lower/io-statement-open-options.f90 +++ b/flang/test/Lower/io-statement-open-options.f90 @@ -15,6 +15,6 @@ subroutine test_convert_specifier(unit) close(unit) end subroutine -! CHECK: fir.global internal @[[be_str_name]] constant : !fir.char<1,10> { +! CHECK: fir.global linkonce @[[be_str_name]] constant : !fir.char<1,10> { ! CHECK: %[[be_str_lit:.*]] = fir.string_lit "BIG_ENDIAN"(10) : !fir.char<1,10> ! CHECK: fir.has_value %[[be_str_lit]] : !fir.char<1,10> diff --git a/flang/test/Lower/namelist.f90 b/flang/test/Lower/namelist.f90 index 6637283..643e30a 100644 --- a/flang/test/Lower/namelist.f90 +++ b/flang/test/Lower/namelist.f90 @@ -83,6 +83,6 @@ subroutine global_pointer write(10, nml=mygroup) end - ! CHECK-DAG: fir.global internal @_QQcl.6A6A6A00 constant : !fir.char<1,4> - ! CHECK-DAG: fir.global internal @_QQcl.63636300 constant : !fir.char<1,4> - ! CHECK-DAG: fir.global internal @_QQcl.6E6E6E00 constant : !fir.char<1,4> + ! CHECK-DAG: fir.global linkonce @_QQcl.6A6A6A00 constant : !fir.char<1,4> + ! CHECK-DAG: fir.global linkonce @_QQcl.63636300 constant : !fir.char<1,4> + ! CHECK-DAG: fir.global linkonce @_QQcl.6E6E6E00 constant : !fir.char<1,4> diff --git a/flang/test/Lower/read-write-buffer.f90 b/flang/test/Lower/read-write-buffer.f90 index 188cdb0..8892092 100644 --- a/flang/test/Lower/read-write-buffer.f90 +++ b/flang/test/Lower/read-write-buffer.f90 @@ -29,7 +29,7 @@ subroutine some() write (buffer, 10) "compiler" read (buffer, 10) greeting end -! CHECK-LABEL: fir.global internal @_QQcl.636F6D70696C6572 +! CHECK-LABEL: fir.global linkonce @_QQcl.636F6D70696C6572 ! CHECK: %[[lit:.*]] = fir.string_lit "compiler"(8) : !fir.char<1,8> ! CHECK: fir.has_value %[[lit]] : !fir.char<1,8> ! CHECK: } diff --git a/flang/unittests/Optimizer/Builder/FIRBuilderTest.cpp b/flang/unittests/Optimizer/Builder/FIRBuilderTest.cpp index 650a4d13..c3436cc 100644 --- a/flang/unittests/Optimizer/Builder/FIRBuilderTest.cpp +++ b/flang/unittests/Optimizer/Builder/FIRBuilderTest.cpp @@ -311,7 +311,7 @@ TEST_F(FIRBuilderTest, createStringLiteral) { auto symbol = addrOp.getSymbol().getRootReference().getValue(); auto global = builder.getNamedGlobal(symbol); EXPECT_EQ( - builder.createInternalLinkage().getValue(), global.getLinkName().value()); + builder.createLinkOnceLinkage().getValue(), global.getLinkName().value()); EXPECT_EQ(fir::CharacterType::get(builder.getContext(), 1, strValue.size()), global.getType()); -- 2.7.4