- User Since
- Sep 16 2016, 10:22 AM (64 w, 4 d)
How would you feel about landing a simplified version of this change that doesn't use the start section (i.e. always put start functions in the metadata)? Seems a little strange to special case the single start function. Also, I like the idea of letting the linker decide if/when to switch to using the start section (for now anyway).
Re-landed as rL320432
Based on the discussion in the bug (https://bugs.llvm.org/show_bug.cgi?id=35533) it sounds like we need to take a look at SHF_LINK_ORDER and how that works. It sounds like its different to the "group" concept. But if we do decide groups are the way to go this change lgtm.
Fri, Dec 8
- update tests
- remove unused member
This can be re-landed one this lld change lands: https://reviews.llvm.org/D40989
- call indirect test
Good call regarding that test, I added it here in a separate change so we can see how this change effects it:
Thu, Dec 7
I create a patch based partially on this one that just does the compression of the function table. https://reviews.llvm.org/D40989.
Hopefully that means this can be rebased into a smaller change.
- remove comment
I think I'm going to abandon this, but the work in this patch and in Nicolas's patch will be probably still end up being generally useful.
Wed, Dec 6
Manually. Is there clang-tidy method of doing this? This change is pretty conservative, it only includes to more obvious cases I could find.
Sorry, didn't mean to imply you were going slow. I just wasn't sure if I was waiting on you at all in this case since I got another LGTM. I can wait :)
In case it wasn't clear I was asking about this change in particular too. Want me to wait on your LG?
Actually if you don't mind I'd like to revert this and prepare a nicer lld-side fix that will work both before and after this change. WDYT?
Looks like this requires a corresponding lld change, broke some tests because we were looking the ->tables() on the wasm objects (which is now empty, since they are in the ->imports() now). I'm looking into a few different ways to fix.
@ruiu, do you prefer that I wait for your LGTM on all lld changes? I'm assuming you would be ok with small-ish wasm-only changes being peer-reviewed rather than requiring owner reviewed? e(assuming you consider yourself the owner of lld?)
I'm gonna see if I can come up with precise test for this, and then will land it.
I'm not quite ready to land this anyway. It needs rebasing and integrating with @ncw proposed version.
Can you update the description.
I'd rather not change the behaviour of --entry since its a flag that is in common usage. I'd also rather not default --no-entry since that means that by default nothing will get pulled into the link (i.e. all users will need explicit --entry and/or --undefined otherwise nothing will be linked).
I wonder if we can add test for this? I guess it doesn't effect the -elf target only the -wasm one?
Tue, Dec 5
Sadly I think we are blocked on: https://github.com/WebAssembly/design/issues/1160.
LGTM - with a couple more comments.
Actually they are weakly referenced in musl:
@ncw , this is change I've be wroking on for a while now that might be help (or an annoying merge conflict!) for your work on function stripping. It basically does for globals what I imagine we would want to do for functions (i.e. eliminates the per file index offset). Sorry if it doesn't merge well with your work, but hopefully there are things that they share and could make your partch smaller?
ping. The sphinx_intro.rst changes are actually real (not just reformatting). I updated them to the cmake world.
- Inline kStackPointer
Thanks for doing this. Looks like a good direction!
- Review feedback
- define kStackPointer inside namespace
Mon, Dec 4
- revert part
Sun, Dec 3
I would argue that we can always add this feature if the future, but implementing now seems unnecessary.
It allows one to write:
I'm not removing --sysroot from the toolchain (i.e. clang and the driver), only from lld. In lld only the ELF driver supports sysroot and the only thing is provides is that ability for the user to pass special -L flag with = at the start. i.e. -L=/path/in/sysroot. Do you think this is useful feature? Normally the sysroot library paths are constructed by clang based on the --sysroot you pass to it, and are fully expanded by the time the linker sees them.
Sat, Dec 2
- remove from .td file