- User Since
- Sep 1 2015, 3:36 AM (194 w, 4 h)
I think it should be possible if you make the relocation point to the object itself (which will have local linkage) and not the alias that you're creating.
Fri, May 17
Could this use a relative relocation so that we aren't paying the cost of a dynamic relocation for each global?
@pcc Sorry I missed your comment
Thu, May 16
I'm still confused. If the behavior intended only has to do with the existence of relocations why are we making this change? A clear difference of behavior needs to be pointed out and that hasn't been made clear.
Wed, May 15
@paulsemel Any comments?
Tue, May 14
Any comments on this?
I'm still a bit unclear. Does it strip symbols that are duplicates of symbols in .dynsym? Does it just get rid of the the whole symbol table no matter what?
Mon, May 13
If we always align first PT_TLS section on 64 bytes wouldn't this fix the problem?
I'm confused as to what this is trying to accomplish.
Wed, May 8
Tue, May 7
I see you originally added it in https://reviews.llvm.org/rL353919.
Does this match GNU objcopy's behaviour? If GNU strips them, we should probably too. If not, we shouldn't.
Mon, May 6
We decide what to internalize after computeDeadSymbols, so why wouldn't we be able to use this to block internalization of linkonce_odr global variables (when !ReadOnly)?
Tue, Apr 30
I agree this doesn't look like the correct fix, especially the check !Sym.isExecutable() which doesn't look correct to me
Looks like I have another idea how this can be fixed. In addition to 'ReadOnly' we can introduce 'CompileTimeConstant' flag in GlobalVarSummary.
Not marking linkonce_odr and weak_odr data symbols which are preserved and are not 'CompileTimeConstant' live will fix PR41645.
Also such flag allows exploring some new optimization opportunities:
Ok, as far as I understood from ongoing discussion, we can first implement --special-syms as a dummy flag. That said, I'm updating the diff according to comments from @jhenderson
Splitted IHEX patch into reader and writer. Diff now contains the "writer" part.
Mon, Apr 29
It isn't working, because llvm::computeDeadSymbols keeps non-prevailing linkonce_odr and weak_odr symbols live.
It is possible to check for function summary when doing this to fix the issue:
if (S->linkage() == GlobalValue::AvailableExternallyLinkage || ((S->linkage() == GlobalValue::WeakODRLinkage || S->linkage() == GlobalValue::LinkOnceODRLinkage) && isa<FunctionSummary>(S->getBaseObject()))) KeepAliveLinkage = true;
however this will prevent optimizing out linkonce_odr and weak_odr constants
Thu, Apr 25
LGTM, thanks. I suggest waiting for 1-2 days in case @tejohnson has something to say.
Wed, Apr 24
There's a lot of code to review here.
I've responded to some of the comments, meanwhile I'm splitting patch into writer (will go first) and reader (will go next). Will update the review soon
Ok, I finally have some time to work on this again
Apr 19 2019
How about making a symlink nm -> llvm_nm and use different behavior depending on tool name?
The same approach is used by llvm-addr2line, AFAIR.
Apr 18 2019
what's wrong with keeping it like it is?
So what are further actions with this? Is emulating GNU nm seems like a better choice?
Apr 17 2019
Can't operand 0 of bitcast (%union.A* %u) be used as a key?
"MutatedMemory" is only used to hold evaluated store values
I wonder if it's possible to stop using MutatedMemory for checks in ComputeLoadResult and instead start using separate map which can also be filled
in EvaluateBlock. In this new map you can use bitcast (or even first operand of bitcast) as a key.
Apr 15 2019
btw, some people at euro llvm also requested srec supprt, which seems extremely similar to ihex
My only concern with this patch: how long does this test take to run, specifically, how long does llvm-mc take to generate a file with this many sections?
Apr 12 2019
Addressed comments from @MaskRay
Looks to me like you've hit a bug here
Okay, thanks. llvm-nm shows them by default, I take it?
FYI, most of the other llvm-objcopy developers are still away following Euro LLVM
Apr 11 2019
Added test case, addressed comments.
That was helpful, thanks! I'll update diff shortly
Apr 10 2019
Apr 8 2019
Apr 5 2019
Line parser moved to IHexRecord::parse
Better test case for reader
Addressed some of review comments
Apr 4 2019
Mar 28 2019
Addressed review comments
Mar 27 2019
New implementation taken from libarchive (dynamic crc table)
Mar 26 2019
It doesn't matter, at most a single memory fence is necessary.
Yep. However, you don't have it in your code. The volatile int crc_tbl_inited prevents compiler optimizations but doesn't prevent out-of-order execution as far as I know.
Shouldn't this be only conditionally compiled if zlib isn't available?
don't bother with the 1KB table in both binary and source, just recompute it on the first use
Mar 19 2019
Mar 18 2019
I suspect that this is going to interact nastily with D59843.
Mar 14 2019
This should be rebased against r354850. Otherwise LGTM.