[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-06 Thread Nick Desaulniers via Phabricator via cfe-commits
nickdesaulniers added a comment. In D68101#1806213 , @rjmccall wrote: > In D68101#1806135 , @nickdesaulniers > wrote: > > > In D68101#1802280 , @rjmccall > > wrote: > > >

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

2020-01-06 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment. In D68101#1806135 , @nickdesaulniers wrote: > In D68101#1802220 , @bd1976llvm > wrote: > > > Below is the code comment from the new patch explaining the new approach, > > please take a

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

2020-01-06 Thread Nick Desaulniers via Phabricator via cfe-commits
nickdesaulniers added a comment. In D68101#1802220 , @bd1976llvm wrote: > Below is the code comment from the new patch explaining the new approach, > please take a look and see if you have any questions/comments: > > // If two globals with differing

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

2020-01-03 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment. In D68101#1804006 , @bd1976llvm wrote: > I looked into these today. I think we can do the first of these. I have put a > WIP patch up here: https://reviews.llvm.org/D72194. Could you comment on the > approach taken? Thank

[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 John McCall via Phabricator via cfe-commits
rjmccall added a comment. 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 to miscompile if they do. It is LLVM's responsibility to correctly compile valid

[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-13 Thread Nick Desaulniers via Phabricator via cfe-commits
nickdesaulniers added a comment. More info: https://docs.oracle.com/cd/E23824_01/html/819-0690/ggdlu.html CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68101/new/ https://reviews.llvm.org/D68101 ___ cfe-commits mailing list

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

2019-12-03 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment. In D68101#1767732 , @nickdesaulniers wrote: > > But ELF allows object files to contain an arbitrary number of what I've > > been calling "section units" that will be assembled into a single section > > in the image. > > More

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

2019-12-03 Thread Nick Desaulniers via Phabricator via cfe-commits
nickdesaulniers added a comment. > But ELF allows object files to contain an arbitrary number of what I've been > calling "section units" that will be assembled into a single section in the > image. More precisely, in assembler, you can specify sections dis-jointly, but they will be rejoined

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

2019-12-03 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment. We're not talking about putting objects in a different section *in the image*. But ELF allows object files to contain an arbitrary number of what I've been calling "section units" that will be assembled into a single section in the image. In theory, we could even

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

2019-12-03 Thread Nick Desaulniers via Phabricator via cfe-commits
nickdesaulniers added a comment. Just some more thoughts after being able to sleep on it. The merge-able sections is generally handy for sections which I'm not specific about (in the added test case, the "implicit" sections). For example, if I have global const data that I don't take the

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

2019-12-03 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment. In D68101#1766670 , @bd1976llvm wrote: > In D68101#1766359 , @rjmccall wrote: > > > I can't speak for the pragma authors, but I strongly doubt the pragma is > > intended to force all

[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 John McCall via Phabricator via cfe-commits
rjmccall added a comment. 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 units is purely an object-file representation issue. Looks to me like

[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-02 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment. > So I don't think we should even be trying to mark sections as mergeable > unless we walk all elements in the section and make sure it's even safe to > apply these. That's the conservative fix, yes. You can either: 1. emit global objects with the same section name

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

2019-12-02 Thread Nick Desaulniers via Phabricator via cfe-commits
nickdesaulniers added a comment. In D68101#1765665 , @rjmccall wrote: > Given all that, this patch seems far too aggressive. While mergeable > sections can be useful for optimizing arbitrary code that might not use a > section, they are also extremely

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

2019-12-02 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment. Let me try to restate what's happening here so that I can see if I understand it. There are two concepts of "section" in play here: - a unit in an object file with a particular section name, which need not be unique within the object file, and which can have

[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