[PATCH] D138463: [windows-itanium] Propagate DLL storage class to Initialisation Guard Variables

2022-11-22 Thread ben via Phabricator via cfe-commits
bd1976llvm added a comment. In D138463#3944124 , @compnerd wrote: > The change itself seems fine, but the CI failure seems related to this change. Thanks. There's been renewed interest in using DLLs with C++ interfaces so we should, hopefully, be able

[PATCH] D138463: [windows-itanium] Propagate DLL storage class to Initialisation Guard Variables

2022-11-22 Thread ben via Phabricator via cfe-commits
bd1976llvm updated this revision to Diff 477287. bd1976llvm edited the summary of this revision. bd1976llvm added a comment. Fix for test failure on *nix. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D138463/new/ https://reviews.llvm.org/D138463 Files:

[PATCH] D133266: [MinGW] Reject explicit hidden visibility applied to dllexport and hidden/protected applied to dllimport

2022-09-27 Thread ben via Phabricator via cfe-commits
bd1976llvm added a comment. Whilst reviewing the impact of these dllimport/export restrictions I noticed that LLVM IR does not reject dllimport/export on local linkage globals. I have put up a PR for adding that restriction: D134784

[PATCH] D133266: [MinGW] Reject explicit hidden visibility applied to dllexport and hidden/protected applied to dllimport

2022-09-12 Thread ben via Phabricator via cfe-commits
bd1976llvm added a comment. In D133266#3776164 , @MaskRay wrote: > In D133266#3776051 , @bd1976llvm > wrote: > >> In D133266#3775832 , @MaskRay >> wrote: >> >>> In

[PATCH] D133266: [MinGW] Reject explicit non-default visibility applied to dllexport/dllimport declaration

2022-09-07 Thread ben via Phabricator via cfe-commits
bd1976llvm added a comment. In D133266#3775832 , @MaskRay wrote: > In D133266#3775384 , @MaskRay wrote: > >> In D133266#3775189 , @bd1976llvm >> wrote: >> >>> >> >>

[PATCH] D133266: [MinGW] Reject explicit non-default visibility applied to dllexport/dllimport declaration

2022-09-07 Thread ben via Phabricator via cfe-commits
bd1976llvm added a comment. This approach doesn't account for the implementation of -fvisibility-global-new-delete-hidden. The following case now fails after this change: > type new_del.cpp namespace std { typedef __typeof__(sizeof(int)) size_t; struct nothrow_t {}; struct align_val_t

[PATCH] D90299: [windows-itanium] handle dllimport/export code paths separately and share with PS4

2021-02-17 Thread ben via Phabricator via cfe-commits
bd1976llvm marked 2 inline comments as done. bd1976llvm added a comment. ping :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90299/new/ https://reviews.llvm.org/D90299 ___ cfe-commits mailing list

[PATCH] D72194: [MC][ELF] Ensure that mergeable globals with an explicit section are assigned to SHF_MERGE sections with compatible entsizes

2020-04-16 Thread ben via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG86478d3de91a: [MC][ELF] Put explicit section name symbols into entry size compatible sections (authored by bd1976llvm). Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository:

[PATCH] D68101: [MC][ELF] Prevent globals with an explicit section from being mergeable

2020-01-13 Thread ben via Phabricator via cfe-commits
bd1976llvm added a comment. In D68101#1806475 , @nickdesaulniers wrote: > In D68101#1806213 , @rjmccall wrote: > > > In D68101#1806135 , > > @nickdesaulniers wrote: > > >

[PATCH] D68101: [MC][ELF] Prevent globals with an explicit section from being mergeable

2020-01-06 Thread ben via Phabricator via cfe-commits
bd1976llvm added a comment. Thanks for the feedback. I'm sorry that I am being slow on this... v.busy at the moment. Will try to finish the patch in the next few days. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68101/new/ https://reviews.llvm.org/D68101

[PATCH] D68101: [MC][ELF] Prevent globals with an explicit section from being mergeable

2020-01-03 Thread ben via Phabricator via cfe-commits
bd1976llvm added a comment. In D68101#1802280 , @rjmccall wrote: > The solution described in that comment is not acceptable. We are not going > to tell users that they cannot assign symbols explicitly to sections because > LLVM is unable to promise not

[PATCH] D68101: [MC][ELF] Prevent globals with an explicit section from being mergeable

2020-01-02 Thread ben via Phabricator via cfe-commits
bd1976llvm added a comment. Sorry for the delay in updating this. I hoped to post an updated patch today for this but I realized that I first need to investigate whether section relative references might interact badly with multiple sections with the same name.. I will be able to update the

[PATCH] D68101: [MC][ELF] Prevent globals with an explicit section from being mergeable

2019-12-03 Thread ben via Phabricator via cfe-commits
bd1976llvm added a comment. In D68101#1766359 , @rjmccall wrote: > I can't speak for the pragma authors, but I strongly doubt the pragma is > intended to force all affected globals to go into a single section unit, > since the division into section

[PATCH] D68101: [MC][ELF] Prevent globals with an explicit section from being mergeable

2019-12-02 Thread ben via Phabricator via cfe-commits
bd1976llvm added a comment. In D68101#1766151 , @nickdesaulniers wrote: > In D68101#1765665 , @rjmccall wrote: > > > Given all that, this patch seems far too aggressive. While mergeable > > sections can be

[PATCH] D68101: [MC][ELF] Prevent globals with an explicit section from being mergeable

2019-12-01 Thread ben via Phabricator via cfe-commits
bd1976llvm added a comment. In D68101#1689622 , @SjoerdMeijer wrote: > Just to make sure I haven't missed anything, I would like to take this patch > and run some numbers, for which I need a little bit of time. Hi @SjoerdMeijer. Did you have a chance

[PATCH] D68147: [MC][ELF] Prevent globals with an explicit section from being mergeable by default and add a safe option to allow mergeable

2019-09-27 Thread ben via Phabricator via cfe-commits
bd1976llvm created this revision. Herald added subscribers: llvm-commits, cfe-commits, hiraditya. Herald added projects: clang, LLVM. Alternative to https://reviews.llvm.org/D68101 Prevents globals with an explicit section from being mergeable by default (as in D68101

[PATCH] D60274: [ELF] Implement Dependent Libraries Feature

2019-05-16 Thread ben via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rLLD360984: [ELF] Implement Dependent Libraries Feature (authored by bd1976llvm, committed by ). Changed prior to commit: https://reviews.llvm.org/D60274?vs=195495=199972#toc Repository: rLLD LLVM

[PATCH] D60274: [ELF] Implement Dependent Libraries Feature

2019-04-25 Thread ben via Phabricator via cfe-commits
bd1976llvm added a comment. I am keen to keep this moving. I think there are a few things outstanding: 1. Need to resolve concerns w.r.t the design. 2. I need to find out whether I should be doing validation when reading the metadata in LLVM. 3. The LLD side needs a more detailed review.

[PATCH] D60274: [ELF] Implement Dependent Libraries Feature

2019-04-16 Thread ben via Phabricator via cfe-commits
bd1976llvm updated this revision to Diff 195495. bd1976llvm added a comment. No longer shortening "dependent libraries" to "deplibs" except for the .deplibs section (as this takes up bytes on disk). CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60274/new/

[PATCH] D60274: [ELF] Implement Dependent Libraries Feature

2019-04-09 Thread ben via Phabricator via cfe-commits
bd1976llvm marked 5 inline comments as done. bd1976llvm added inline comments. Comment at: lld/ELF/InputFiles.cpp:402 + if (Config->DepLibs) +if (fs::exists(Specifier)) + Driver->addFile(Specifier, /*WithLOption=*/false); jyknight wrote: > bd1976llvm

[PATCH] D60274: [ELF] Implement Dependent Libraries Feature

2019-04-09 Thread ben via Phabricator via cfe-commits
bd1976llvm marked 25 inline comments as done. bd1976llvm added a comment. I have updated the diff to address review comments. I think we can continue to refine this patch in parallel with discussing the design further (I am not dismissing the concerns raised by @compnerd and @jyknight). I am

[PATCH] D53263: Fix places where the return type of a FunctionDecl was being used in place of the function type

2018-11-11 Thread Ben via Phabricator via cfe-commits
bobsayshilol added a comment. In https://reviews.llvm.org/D53263#1294477, @kristina wrote: > Huge apologies, it seems I can't get this to patch cleanly against my fork > and therefore can't test it before committing, which is something I generally > always do. I'll leave it to someone else.

[PATCH] D53263: Fix places where the return type of a FunctionDecl was being used in place of the function type

2018-11-10 Thread Ben via Phabricator via cfe-commits
bobsayshilol added a comment. In https://reviews.llvm.org/D53263#1294383, @kristina wrote: > I can do it if you'd like, will be a moment though. Thanks and much appreciated! https://reviews.llvm.org/D53263 ___ cfe-commits mailing list

[PATCH] D53263: Fix places where the return type of a FunctionDecl was being used in place of the function type

2018-11-10 Thread Ben via Phabricator via cfe-commits
bobsayshilol added a comment. Thanks! I don't have commit access to land this myself. https://reviews.llvm.org/D53263 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D53263: Fix places where the return type of a FunctionDecl was being used in place of the function type

2018-11-08 Thread Ben via Phabricator via cfe-commits
bobsayshilol added a reviewer: rjmccall. bobsayshilol added a comment. Ping. https://reviews.llvm.org/D53263 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D53263: Fix places where the return type of a FunctionDecl was being used in place of the function type

2018-10-27 Thread Ben via Phabricator via cfe-commits
bobsayshilol added a comment. Ping. https://reviews.llvm.org/D53263 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D53263: Fix places where the return type of a FunctionDecl was being used in place of the function type

2018-10-19 Thread Ben via Phabricator via cfe-commits
bobsayshilol updated this revision to Diff 170277. bobsayshilol retitled this revision from "[CodeGen] Fix places where the return type of a FunctionDecl was being used in place of the function type" to "Fix places where the return type of a FunctionDecl was being used in place of the function

[PATCH] D53263: [CodeGen] Fix places where the return type of a FunctionDecl was being used in place of the function type

2018-10-18 Thread Ben via Phabricator via cfe-commits
bobsayshilol updated this revision to Diff 170136. bobsayshilol edited the summary of this revision. bobsayshilol added a comment. Fixed the added assert to do the right thing. Clang can now build with a debug Clang built from this patch, and all unittests I could find pass in that built

[PATCH] D53263: [CodeGen] Fix places where the return type of a FunctionDecl was being used in place of the function type

2018-10-14 Thread Ben via Phabricator via cfe-commits
bobsayshilol created this revision. bobsayshilol added reviewers: sberg, ahatanak. Herald added subscribers: cfe-commits, jfb. `FunctionDecl::Create()` takes as its `T` parameter the type of function that should be created, not the return type. Passing in the return type looks to have been

[PATCH] D50101: [asan] Update a vector's storage annotation during destruction.

2018-08-25 Thread Ben via Phabricator via cfe-commits
bobsayshilol added a comment. Pinging this as it's been "Ready to Land" for a while and I want to check that that's not because I've forgotten to do something? Repository: rCXX libc++ https://reviews.llvm.org/D50101 ___ cfe-commits mailing list

[PATCH] D50101: [asan] Update a vector's storage annotation during destruction.

2018-08-01 Thread Ben via Phabricator via cfe-commits
bobsayshilol added a comment. Thanks! I don't have write access to svn so I can't commit the patch myself, but assuming that I've read the docs correctly if anyone is able to take it I'd be grateful. Repository: rCXX libc++ https://reviews.llvm.org/D50101

[PATCH] D50101: [asan] Update a vector's storage annotation during destruction.

2018-07-31 Thread Ben via Phabricator via cfe-commits
bobsayshilol created this revision. bobsayshilol added reviewers: mclow.lists, EricWF. Herald added subscribers: cfe-commits, ldionne. This allows a user replaced operator delete to modify or reuse returned memory in the case where the size and capacity of a vector do not match upon destruction