[PATCH] D158538: [MS-ABI] Remove comdat attribute for inheriting ctor.

2023-08-28 Thread Jennifer Yu via Phabricator via cfe-commits
jyu2 marked an inline comment as done. jyu2 added a comment. Thanks. @rnk Comment at: clang/test/CodeGenCXX/ms-inheriting-ctor.cpp:41 + +// CHECK-LABEL: define internal noundef ptr @"??0?$B@_N@@QEAA@AEBVF@@AEBUA@@@Z"(ptr noundef nonnull returned align 1 dereferenceable(1)

[PATCH] D158538: [MS-ABI] Remove comdat attribute for inheriting ctor.

2023-08-28 Thread Jennifer Yu via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG1d0bd8e51be2: [MSABI] Remove comdat attribute for inheriting ctor. (authored by jyu2). Changed prior to commit:

[PATCH] D158538: [MS-ABI] Remove comdat attribute for inheriting ctor.

2023-08-28 Thread Reid Kleckner via Phabricator via cfe-commits
rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm with some tweaks to the test. Comment at: clang/test/CodeGenCXX/ms-inheriting-ctor.cpp:41 + +// CHECK-LABEL: define internal noundef ptr

[PATCH] D158538: [MS-ABI] Remove comdat attribute for inheriting ctor.

2023-08-28 Thread Jennifer Yu via Phabricator via cfe-commits
jyu2 updated this revision to Diff 554025. jyu2 added a comment. Thanks @rnk. This is address his comment. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158538/new/ https://reviews.llvm.org/D158538 Files: clang/lib/AST/ASTContext.cpp

[PATCH] D158538: [MS-ABI] Remove comdat attribute for inheriting ctor.

2023-08-28 Thread Jennifer Yu via Phabricator via cfe-commits
jyu2 added a comment. Hi @rnk, Thank you so much for the suggestion. >> Before we do that, you should be able to delete the logic in >> CodeGenModule::getFunctionLinkage to handle inheriting constructor thunks. >> Your change should have the same effect. Can you do that, and check that it >>

[PATCH] D158538: [MS-ABI] Remove comdat attribute for inheriting ctor.

2023-08-28 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment. It took me a while to find this code to understand the issue. The problem is that there is an inconsistency between the GVA linkage and the LLVM IR linkage for these constructors. In this godbolt example , `??0?$TInputDataVertexModel..`

[PATCH] D158538: [MS-ABI] Remove comdat attribute for inheriting ctor.

2023-08-28 Thread Jennifer Yu via Phabricator via cfe-commits
jyu2 added a comment. Ping, Our customer fond problem I submit bug: in https://github.com/llvm/llvm-project/issues/65045 This issue related with big change of 5179eb78210a2ad01a18c37b75048ccfe78414ac Where internal linkage for inheriting ctor were set in change set at following code. Could

[PATCH] D158538: [MS-ABI] Remove comdat attribute for inheriting ctor.

2023-08-22 Thread Jennifer Yu via Phabricator via cfe-commits
jyu2 created this revision. jyu2 added reviewers: rnk, rsmith, majnemer, cfe-commits. jyu2 added a project: clang. Herald added a project: All. jyu2 requested review of this revision. Currently, for MS, the linkage for the inheriting constructors is set to internal. However, the comdat attribute