This is an archive of the discontinued LLVM Phabricator instance.

Differential D58807

[CodeGen] COMDAT-fold block descriptors
ClosedPublic

Authored by DHowett-MSFT on Feb 28 2019, 6:09 PM.

Details

Reviewers
theraven
Commits
rG17d429535984: COMDAT-fold block descriptors.
rC357363: COMDAT-fold block descriptors.
rL357363: COMDAT-fold block descriptors.
Summary

Without this change, linking multiple objects containing block
descriptors together on Windows will generate duplicate symbol errors.

Diff Detail

Repository
rL LLVM

Event Timeline

DHowett-MSFT created this revision.Feb 28 2019, 6:09 PM
Herald added a subscriber: cfe-commits. · View Herald TranscriptFeb 28 2019, 6:09 PM
theraven accepted this revision.Mar 1 2019, 12:34 AM
theraven added a subscriber: theraven.

Looks good to me. On ELF and Mach-O, I think we get the equivalent behaviour automatically from the ODR linkage type. I'd really like to see linkage type and COMDAT made orthogonal, but that's a bigger change.

This revision is now accepted and ready to land.Mar 1 2019, 12:34 AM
theraven added a comment.Mar 1 2019, 12:35 AM

It would probably benefit from a test so that we don't regress.

DHowett-MSFT updated this revision to Diff 189183.Mar 4 2019, 12:05 PM

This change caused a test to fail, so I took the opportunity to augment the test with COMDAT entry.

DHowett-MSFT updated this revision to Diff 189193.Mar 4 2019, 1:14 PM

Fixed corrupted patch. I know that the -str test case is not the _ideal_ location for the comdat test, but it is the only test whose invariants failed (the IR for the descriptor definition now contains comdat, align 4).

Closed by commit rL357363: COMDAT-fold block descriptors. (authored by theraven). · Explain WhyMar 31 2019, 4:21 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptMar 31 2019, 4:21 AM
Herald added a subscriber: llvm-commits. · View Herald Transcript

Revision Contents

PathSize
cfe/
trunk/
lib/
CodeGen/
2 lines
test/
CodeGenObjC/
8 lines

Diff 193007

cfe/trunk/lib/CodeGen/CGBlocks.cpp

Show First 20 Lines • Show All 268 Lines • ▼ Show 20 Lines if (descName.empty()) {
linkage = llvm::GlobalValue::LinkOnceODRLinkage; linkage = llvm::GlobalValue::LinkOnceODRLinkage;
} }
llvm::GlobalVariable *global = llvm::GlobalVariable *global =
elements.finishAndCreateGlobal(descName, CGM.getPointerAlign(), elements.finishAndCreateGlobal(descName, CGM.getPointerAlign(),
/*constant*/ true, linkage, AddrSpace); /*constant*/ true, linkage, AddrSpace);
if (linkage == llvm::GlobalValue::LinkOnceODRLinkage) { if (linkage == llvm::GlobalValue::LinkOnceODRLinkage) {
if (CGM.supportsCOMDAT())
global->setComdat(CGM.getModule().getOrInsertComdat(descName));
global->setVisibility(llvm::GlobalValue::HiddenVisibility); global->setVisibility(llvm::GlobalValue::HiddenVisibility);
global->setUnnamedAddr(llvm::GlobalValue::UnnamedAddr::Global); global->setUnnamedAddr(llvm::GlobalValue::UnnamedAddr::Global);
} }
return llvm::ConstantExpr::getBitCast(global, CGM.getBlockDescriptorType()); return llvm::ConstantExpr::getBitCast(global, CGM.getBlockDescriptorType());
} }
/* /*
▲ Show 20 Lines • Show All 2,741 Lines • Show Last 20 Lines

cfe/trunk/test/CodeGenObjC/block-desc-str.m

// RUN: %clang_cc1 -triple x86_64-unknown-freebsd -emit-llvm -fobjc-runtime=gnustep-1.7 -fblocks -o - %s | FileCheck %s // RUN: %clang_cc1 -triple x86_64-unknown-freebsd -emit-llvm -fobjc-runtime=gnustep-1.7 -fblocks -o - %s | FileCheck --check-prefix=CHECK-COMDAT %s
// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -emit-llvm -fobjc-runtime=gcc -fblocks -o - %s | FileCheck %s // RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -emit-llvm -fobjc-runtime=gcc -fblocks -o - %s | FileCheck --check-prefix=CHECK-COMDAT %s
// RUN: %clang_cc1 -triple=x86_64-apple-darwin10 -emit-llvm -fblocks -o - %s | FileCheck %s // RUN: %clang_cc1 -triple=x86_64-apple-darwin10 -emit-llvm -fblocks -o - %s | FileCheck %s
// Test that descriptor symbol names don't include '@'. // Test that descriptor symbol names don't include '@'.
// CHECK-COMDAT: $"__block_descriptor_40_8_32o_e5_v8\01?0l" = comdat any
// CHECK-COMDAT: @[[STR:.*]] = private unnamed_addr constant [6 x i8] c"v8@?0\00"
// CHECK-COMDAT: @"__block_descriptor_40_8_32o_e5_v8\01?0l" = linkonce_odr hidden unnamed_addr constant { i64, i64, i8*, i8*, i8*, {{.*}} } { i64 0, i64 40, i8* bitcast ({{.*}} to i8*), i8* bitcast ({{.*}} to i8*), i8* getelementptr inbounds ([6 x i8], [6 x i8]* @[[STR]], i32 0, i32 0), {{.*}} }, comdat, align 8
// CHECK: @[[STR:.*]] = private unnamed_addr constant [6 x i8] c"v8@?0\00" // CHECK: @[[STR:.*]] = private unnamed_addr constant [6 x i8] c"v8@?0\00"
// CHECK: @"__block_descriptor_40_8_32o_e5_v8\01?0l" = linkonce_odr hidden unnamed_addr constant { i64, i64, i8*, i8*, i8*, {{.*}} } { i64 0, i64 40, i8* bitcast ({{.*}} to i8*), i8* bitcast ({{.*}} to i8*), i8* getelementptr inbounds ([6 x i8], [6 x i8]* @[[STR]], i32 0, i32 0), {{.*}} }, align 8 // CHECK: @"__block_descriptor_40_8_32o_e5_v8\01?0l" = linkonce_odr hidden unnamed_addr constant { i64, i64, i8*, i8*, i8*, {{.*}} } { i64 0, i64 40, i8* bitcast ({{.*}} to i8*), i8* bitcast ({{.*}} to i8*), i8* getelementptr inbounds ([6 x i8], [6 x i8]* @[[STR]], i32 0, i32 0), {{.*}} }, align 8
typedef void (^BlockTy)(void); typedef void (^BlockTy)(void);
void test(id a) { void test(id a) {
BlockTy b = ^{ (void)a; }; BlockTy b = ^{ (void)a; };
} }