[PATCH] D55229: [COFF] Statically link certain runtime library functions

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit rC359513: Re-land r359250, [COFF] Statically link certain runtime library functions (authored by rnk, committed by ).

[PATCH] D55229: [COFF] Statically link certain runtime library functions

rnk updated this revision to Diff 197211. rnk added a comment. - update test to check IR Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55229/new/ https://reviews.llvm.org/D55229 Files: clang/lib/CodeGen/CodeGenModule.cpp

[PATCH] D55229: [COFF] Statically link certain runtime library functions

rnk added a comment. In D55229#1479712 , @jfb wrote: > Looks like this might have broken bots: I replied on Thursday, but it looks like it didn't make it to Phab: > I thought I reverted it, did I not push that? Looking now, it seems I did push the

[PATCH] D55229: [COFF] Statically link certain runtime library functions

jfb added a comment. Looks like this might have broken bots: /Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/llvm/tools/clang/test/CodeGenCXX/runtime-dllstorage.cpp:121:26: error: CHECK-IA-DAG: expected string not found in input // CHECK-DYNAMIC-IA-DAG: declare

[PATCH] D55229: [COFF] Statically link certain runtime library functions

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit rL359250: [COFF] Statically link certain runtime library functions (authored by rnk, committed by ). Herald added a

[PATCH] D55229: [COFF] Statically link certain runtime library functions

rnk updated this revision to Diff 196553. rnk added a comment. Herald added a project: clang. - rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55229/new/ https://reviews.llvm.org/D55229 Files: clang/lib/CodeGen/CodeGenModule.cpp

[PATCH] D55229: [COFF] Statically link certain runtime library functions

rnk commandeered this revision. rnk edited reviewers, added: mgrang; removed: rnk. rnk added a comment. In D55229#1455472 , @rnk wrote: > This patch fixes a lot of LNK4286 warnings when running `check-asan` on > Windows, so I'd like to get it committed

[PATCH] D55229: [COFF] Statically link certain runtime library functions

rnk added a comment. This patch fixes a lot of LNK4286 warnings when running `check-asan` on Windows, so I'd like to get it committed upstream. Are there any remaining objections? Is it OK if I commandeer the revision and make some minor aesthetic adjustments and land it? CHANGES SINCE LAST

[PATCH] D55229: [COFF] Statically link certain runtime library functions

abdulras added a comment. @rnk, I agree that it is a performance optimization, and it actually is in the hot path. I can understand the motivation for the jump. I think that the trampoline might also be a problem for lldb as it (or used to?) statically analyzes to see if it is jumping to

[PATCH] D55229: [COFF] Statically link certain runtime library functions

rnk added inline comments. Comment at: CodeGenObjC/gnu-init.m:103 +// Make sure we do not have dllimport on the load function +// CHECK-WIN: declare dso_local void @__objc_load theraven wrote: > compnerd wrote: > > rnk wrote: > > > compnerd wrote: > > > > rnk

[PATCH] D55229: [COFF] Statically link certain runtime library functions

theraven added a comment. If we're going to change the default, please at least add a flag to allow callers to opt into dlimport. We can then make that dependent on `-static-objc` or similar. Comment at: CodeGenObjC/gnu-init.m:103 +// Make sure we do not have dllimport on

[PATCH] D55229: [COFF] Statically link certain runtime library functions

compnerd added inline comments. Comment at: CodeGen/CodeGenModule.cpp:2957-2958 !getCodeGenOpts().LTOVisibilityPublicStd && - !getTriple().isWindowsGNUEnvironment()) { + !getTriple().isWindowsGNUEnvironment() && +

[PATCH] D55229: [COFF] Statically link certain runtime library functions

rnk added a comment. Thanks. I think what we really want to do here is reconsider our default for applying dllimport. Leaving things unannotated is a good safe default for every environment. In the absence of any flags, clang should assume runtime functions are statically linked. The linker

[PATCH] D55229: [COFF] Statically link certain runtime library functions

compnerd added a subscriber: theraven. compnerd added inline comments. Comment at: CodeGen/CodeGenModule.cpp:2957-2958 !getCodeGenOpts().LTOVisibilityPublicStd && - !getTriple().isWindowsGNUEnvironment()) { + !getTriple().isWindowsGNUEnvironment() &&

[PATCH] D55229: [COFF] Statically link certain runtime library functions

rnk added a comment. Sorry, I was out sick for a week. We should ask @smeenai about this change, since he has been doing more work in this area recently. Comment at: CodeGen/CodeGenModule.cpp:2957-2958 !getCodeGenOpts().LTOVisibilityPublicStd && -

[PATCH] D55229: [COFF] Statically link certain runtime library functions

mgrang added a comment. Ping for reviews please. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55229/new/ https://reviews.llvm.org/D55229 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D55229: [COFF] Statically link certain runtime library functions

mgrang marked an inline comment as done. mgrang added inline comments. Comment at: CodeGenObjC/gnu-init.m:103 +// Make sure we do not have dllimport on the load function +// CHECK-WIN: declare dso_local void @__objc_load I had to remove dllimport in this and

[PATCH] D55229: [COFF] Statically link certain runtime library functions

mgrang updated this revision to Diff 176693. mgrang retitled this revision from "[COFF, ARM64] Make -flto-visibility-public-std a driver and cc1 flag" to "[COFF] Statically link certain runtime library functions". mgrang edited the summary of this revision. CHANGES SINCE LAST ACTION