Skip to content

Commit

Permalink
[flang] Add comdats to functions with linkonce linkage (#66516)
Browse files Browse the repository at this point in the history
This fixes a bug where functions generated by the MLIR Math dialect, for
example ipowi, would fail to link with link.exe on Windows due to having
linkonce linkage but no associated comdat. Adding the comdat on ELF also
allows linkers to perform better garbage collection in the binary.

Simply adding comdats to all functions with this linkage type should
also cover future cases where linkonce or linkonce_odr functions might
be necessary.

(cherry picked from commit 5f476b8)
  • Loading branch information
DavidTruby authored and tru committed Oct 31, 2023
1 parent bdb1553 commit 12bbcd6
Show file tree
Hide file tree
Showing 3 changed files with 57 additions and 1 deletion.
9 changes: 9 additions & 0 deletions flang/lib/Optimizer/CodeGen/CodeGen.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
#include "mlir/Conversion/ReconcileUnrealizedCasts/ReconcileUnrealizedCasts.h"
#include "mlir/Conversion/VectorToLLVM/ConvertVectorToLLVM.h"
#include "mlir/Dialect/LLVMIR/LLVMDialect.h"
#include "mlir/Dialect/LLVMIR/Transforms/AddComdats.h"
#include "mlir/Dialect/OpenACC/OpenACC.h"
#include "mlir/Dialect/OpenMP/OpenMPDialect.h"
#include "mlir/IR/BuiltinTypes.h"
Expand Down Expand Up @@ -3815,6 +3816,14 @@ class FIRToLLVMLowering
std::move(pattern)))) {
signalPassFailure();
}

// Run pass to add comdats to functions that have weak linkage on relevant platforms
if (fir::getTargetTriple(mod).supportsCOMDAT()) {
mlir::OpPassManager comdatPM("builtin.module");
comdatPM.addPass(mlir::LLVM::createLLVMAddComdats());
if (mlir::failed(runPipeline(comdatPM, mod)))
return signalPassFailure();
}
}

private:
Expand Down
41 changes: 41 additions & 0 deletions flang/test/Fir/comdat.fir
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@

// RUN: fir-opt %s --fir-to-llvm-ir="target=x86_64-unknown-linux-gnu" | FileCheck %s --check-prefixes="CHECK-COMDAT"
// RUN: fir-opt %s --fir-to-llvm-ir="target=x86_64-pc-windows-msvc" | FileCheck %s --check-prefixes="CHECK-COMDAT"
// RUN: fir-opt %s --fir-to-llvm-ir="target=aarch64-apple-darwin" | FileCheck %s --check-prefixes="CHECK-NOCOMDAT"
// RUN: fir-opt %s --fir-to-llvm-ir="target=powerpc64-ibm-aix" | FileCheck %s --check-prefixes="CHECK-NOCOMDAT"

// CHECK-COMDAT: llvm.func linkonce @fun_linkonce(%arg0: i32) -> i32 comdat(@__llvm_comdat::@fun_linkonce)
// CHECK-NOCOMDAT: llvm.func linkonce @fun_linkonce(%arg0: i32) -> i32 {
func.func @fun_linkonce(%arg0: i32) -> i32 attributes {llvm.linkage = #llvm.linkage<linkonce>} {
return %arg0 : i32
}

// CHECK-COMDAT: llvm.func linkonce_odr @fun_linkonce_odr(%arg0: i32) -> i32 comdat(@__llvm_comdat::@fun_linkonce_odr)
// CHECK-NOCOMDAT: llvm.func linkonce_odr @fun_linkonce_odr(%arg0: i32) -> i32 {
func.func @fun_linkonce_odr(%arg0: i32) -> i32 attributes {llvm.linkage = #llvm.linkage<linkonce_odr>} {
return %arg0 : i32
}

// CHECK-COMDAT: llvm.mlir.global linkonce constant @global_linkonce() comdat(@__llvm_comdat::@global_linkonce) {addr_space = 0 : i32} : i32
// CHECK-NOCOMDAT: llvm.mlir.global linkonce constant @global_linkonce() {addr_space = 0 : i32} : i32 {
fir.global linkonce @global_linkonce constant : i32 {
%0 = arith.constant 0 : i32
fir.has_value %0 : i32
}

// CHECK-COMDAT: llvm.comdat @__llvm_comdat {
// CHECK-COMDAT: llvm.comdat_selector @fun_linkonce_odr any
// CHECK-COMDAT: llvm.comdat_selector @fun_linkonce any
// CHECK-COMDAT: llvm.comdat_selector @global_linkonce any
// CHECK-COMDAT: llvm.comdat_selector @global_linkonce_odr any

// CHECK-NOCOMDAT-NOT: llvm.comdat
// CHECK-NOCOMDAT-NOT: llvm.comdat_selector


// CHECK-COMDAT: llvm.mlir.global linkonce_odr constant @global_linkonce_odr() comdat(@__llvm_comdat::@global_linkonce_odr) {addr_space = 0 : i32} : i32
// CHECK-NOCOMDAT: llvm.mlir.global linkonce_odr constant @global_linkonce_odr() {addr_space = 0 : i32} : i32 {
fir.global linkonce_odr @global_linkonce_odr constant : i32 {
%0 = arith.constant 0 : i32
fir.has_value %0 : i32
}
8 changes: 7 additions & 1 deletion flang/test/Intrinsics/math-codegen.fir
Original file line number Diff line number Diff line change
Expand Up @@ -1467,9 +1467,15 @@ func.func private @llvm.powi.f64.i32(f64, i32) -> f64
func.func private @pow(f64, f64) -> f64

//--- exponentiation_integer.fir
// RUN: fir-opt %t/exponentiation_integer.fir --fir-to-llvm-ir="target=x86_64-unknown-linux-gnu" | FileCheck %t/exponentiation_integer.fir
// RUN: fir-opt %t/exponentiation_integer.fir --fir-to-llvm-ir="target=x86_64-unknown-linux-gnu" | FileCheck %t/exponentiation_integer.fir --check-prefixes="CHECK,CHECK-COMDAT"
// RUN: fir-opt %t/exponentiation_integer.fir --fir-to-llvm-ir="target=x86_64-pc-windows-msvc" | FileCheck %t/exponentiation_integer.fir --check-prefixes="CHECK,CHECK-COMDAT"
// RUN: fir-opt %t/exponentiation_integer.fir --fir-to-llvm-ir="target=aarch64-apple-darwin" | FileCheck %t/exponentiation_integer.fir --check-prefixes="CHECK,CHECK-NOCOMDAT"
// CHECK-COMDAT: llvm.comdat_selector @__mlir_math_ipowi_i32 any
// CHECK-NOCOMDAT-NOT: llvm.comdat_selector @__mlir_math_ipowi_i32 any
// CHECK: @_QPtest_int4
// CHECK: llvm.call @__mlir_math_ipowi_i32({{%[A-Za-z0-9._]+}}, {{%[A-Za-z0-9._]+}}) : (i32, i32) -> i32
// CHECK-COMDAT: llvm.func linkonce_odr @__mlir_math_ipowi_i32(%arg0: i32, %arg1: i32) -> i32 comdat(@__llvm_comdat::@__mlir_math_ipowi_i32)
// CHECK-NOCOMDAT: llvm.func linkonce_odr @__mlir_math_ipowi_i32(%arg0: i32, %arg1: i32) -> i32

func.func @_QPtest_int4(%arg0: !fir.ref<i32> {fir.bindc_name = "x"}, %arg1: !fir.ref<i32> {fir.bindc_name = "y"}, %arg2: !fir.ref<i32> {fir.bindc_name = "z"}) {
%0 = fir.load %arg0 : !fir.ref<i32>
Expand Down

0 comments on commit 12bbcd6

Please sign in to comment.