ncw (Nicholas Wilson)
User

Projects

User does not belong to any projects.

User Details

User Since
Nov 24 2017, 7:16 AM (7 w, 5 d)

Recent Activity

Today

ncw added a comment to D42030: [WebAssembly] Define __heap_base global.

Also, I don't really understand your logic regarding the size of your bitmap. Surely the larger the pages, the fewer pages exist, the smaller the bitmap required to track them? Perhaps I'm missing something?

Wed, Jan 17, 2:55 PM
ncw added inline comments to D42105: [WebAssembly] Symbol changes #4: export relocatable, LLD only.
Wed, Jan 17, 2:38 PM
ncw added a comment to D42193: [WebAssembly] Remove DEBUG_FUNCTION_NAME after llvm change.

Thanks for splitting. It would be an extremely long chain of patches if I submitted them in chunks this small - I hope the chunks I'm using are roughly the right size for sending to you to (I could go bigger or smaller).

Wed, Jan 17, 2:28 PM
ncw added a comment to D42075: [WebAssembly] Symbol changes #1, LLVM.

Thanks for the reviews! I created a fair amount of work for you with these.

Wed, Jan 17, 10:08 AM
ncw added a comment to D42030: [WebAssembly] Define __heap_base global.

In fact, I wonder now as I re-do the calculations if I shouldn't in fact use 16KiB page size (reducing wastage to 10% for 129KiB allocations, at the expense of having a 16KiB mmap bitmap.)

Wed, Jan 17, 9:20 AM
ncw added inline comments to D42030: [WebAssembly] Define __heap_base global.
Wed, Jan 17, 9:13 AM
ncw updated the diff for D41955: [WebAssembly] Symbol changes #5: add "symbol index" concept (LLD).

Updated:

  • Simple rebase to fix conflict with __heap_base change
Wed, Jan 17, 7:52 AM
ncw updated the diff for D42105: [WebAssembly] Symbol changes #4: export relocatable, LLD only.

Updated:

  • Fixed test after rebase on top of __heap_base change
Wed, Jan 17, 7:42 AM
ncw added inline comments to D42105: [WebAssembly] Symbol changes #4: export relocatable, LLD only.
Wed, Jan 17, 7:25 AM
ncw added inline comments to D41510: [WebAssembly] MC: Fix for address taken aliases.
Wed, Jan 17, 6:34 AM
ncw updated the diff for D42176: [WebAssembly] Optimise relocation iteration to remove n^2 loop (LLD).

Updated:

  • Remove copy for already-sorted input
Wed, Jan 17, 6:24 AM
ncw created D42176: [WebAssembly] Optimise relocation iteration to remove n^2 loop (LLD).
Wed, Jan 17, 6:17 AM
ncw updated the diff for D41954: [WebAssembly] Symbol changes #5: add "symbol index" concept (LLVM).

Updated:

  • Simple rebase to fix conflict
Wed, Jan 17, 5:30 AM
ncw updated the diff for D42075: [WebAssembly] Symbol changes #1, LLVM.

Updated:

  • Reverted change to remove imports from NAME section. They are now written out as before.
Wed, Jan 17, 4:31 AM
ncw added a comment to D42075: [WebAssembly] Symbol changes #1, LLVM.

I think dropping the debug names from the symbol table seems like a good idea.
I'm not so sure about dropping the the imports from the name section. It seems at least like a separate change.
I like added requirement on the section ordering, although I'd prefer a more explicit way to check for it.

What do you think about dropping the name section completely from the compiler output? If the compiler output is meant mainly as a linker input perhaps we don't need a names section at all (since its arguably redundant in the face of the symbol table stuff).

Wed, Jan 17, 4:30 AM
ncw updated the diff for D42076: [WebAssembly] Symbol changes #1, LLD.

Updated:

  • Revert change to remove imports from NAME section; now names are written out for imported functions as before
Wed, Jan 17, 4:30 AM
ncw updated the summary of D41954: [WebAssembly] Symbol changes #5: add "symbol index" concept (LLVM).
Wed, Jan 17, 2:59 AM
ncw updated the summary of D42117: [WebAssembly] Symbol changes #6: syminfo index, LLVM.
Wed, Jan 17, 2:50 AM
ncw updated the diff for D42096: [WebAssembly] Symbol changes #3: Cosmetic table, LLD. NFC..

Updated:

  • Renamed constant
Wed, Jan 17, 1:36 AM
ncw added inline comments to D42081: [WebAssembly] Symbol changes #2: Table relocs, LLD.
Wed, Jan 17, 1:35 AM
ncw updated the diff for D42081: [WebAssembly] Symbol changes #2: Table relocs, LLD.

Updated:

  • Responded to comments (formatting/tidying changes)
Wed, Jan 17, 1:35 AM
ncw added inline comments to D42080: [WebAssembly] Symbol changes #2: Table relocs, LLVM.
Wed, Jan 17, 12:54 AM
ncw added inline comments to D42030: [WebAssembly] Define __heap_base global.
Wed, Jan 17, 12:42 AM

Yesterday

ncw updated the summary of D42117: [WebAssembly] Symbol changes #6: syminfo index, LLVM.
Tue, Jan 16, 10:45 AM
ncw created D42118: [WebAssembly] Symbol changes #6: syminfo index, LLD.
Tue, Jan 16, 10:44 AM
ncw created D42117: [WebAssembly] Symbol changes #6: syminfo index, LLVM.
Tue, Jan 16, 10:43 AM
ncw added inline comments to D41955: [WebAssembly] Symbol changes #5: add "symbol index" concept (LLD).
Tue, Jan 16, 9:56 AM
ncw added inline comments to D41954: [WebAssembly] Symbol changes #5: add "symbol index" concept (LLVM).
Tue, Jan 16, 9:53 AM
ncw updated the diff for D41955: [WebAssembly] Symbol changes #5: add "symbol index" concept (LLD).

Updated: split up the big review into six smaller ones.

Tue, Jan 16, 9:49 AM
ncw updated the diff for D41954: [WebAssembly] Symbol changes #5: add "symbol index" concept (LLVM).

Updated: split up the big review into six smaller ones. This is the core change still, the change that motivated it all!

Tue, Jan 16, 9:43 AM
ncw updated the diff for D42105: [WebAssembly] Symbol changes #4: export relocatable, LLD only.

Updated: tiny tweak to calculateImports to fix up case of unresolved weak globals

Tue, Jan 16, 9:21 AM
ncw added inline comments to D42105: [WebAssembly] Symbol changes #4: export relocatable, LLD only.
Tue, Jan 16, 7:19 AM
ncw updated the diff for D42105: [WebAssembly] Symbol changes #4: export relocatable, LLD only.

Updated: Fix mistake not exporting weak flag

Tue, Jan 16, 7:18 AM
ncw created D42105: [WebAssembly] Symbol changes #4: export relocatable, LLD only.
Tue, Jan 16, 7:10 AM
ncw updated the summary of D42080: [WebAssembly] Symbol changes #2: Table relocs, LLVM.
Tue, Jan 16, 3:26 AM
ncw added inline comments to D42096: [WebAssembly] Symbol changes #3: Cosmetic table, LLD. NFC..
Tue, Jan 16, 3:21 AM
ncw added inline comments to D42095: [WebAssembly] Symbol changes #3: Cosmetic table, LLVM. NFC..
Tue, Jan 16, 3:20 AM
ncw updated the summary of D42095: [WebAssembly] Symbol changes #3: Cosmetic table, LLVM. NFC..
Tue, Jan 16, 3:16 AM
ncw created D42096: [WebAssembly] Symbol changes #3: Cosmetic table, LLD. NFC..
Tue, Jan 16, 3:16 AM
ncw created D42095: [WebAssembly] Symbol changes #3: Cosmetic table, LLVM. NFC..
Tue, Jan 16, 3:16 AM

Mon, Jan 15

ncw added a comment to D42030: [WebAssembly] Define __heap_base global.

Nice, thank you! My Musl port makes use of this, I'll test it sometime and confirm it's working. I do need to get onto that and try and work out how to make it useful to the Emscripten folks or the compat waterfall.

Mon, Jan 15, 1:55 PM
ncw added inline comments to D42080: [WebAssembly] Symbol changes #2: Table relocs, LLVM.
Mon, Jan 15, 10:55 AM
ncw updated the summary of D42080: [WebAssembly] Symbol changes #2: Table relocs, LLVM.
Mon, Jan 15, 10:55 AM
ncw created D42081: [WebAssembly] Symbol changes #2: Table relocs, LLD.
Mon, Jan 15, 10:54 AM
ncw created D42080: [WebAssembly] Symbol changes #2: Table relocs, LLVM.
Mon, Jan 15, 10:48 AM
ncw updated the summary of D42075: [WebAssembly] Symbol changes #1, LLVM.
Mon, Jan 15, 9:08 AM
ncw created D42076: [WebAssembly] Symbol changes #1, LLD.
Mon, Jan 15, 9:07 AM
ncw created D42075: [WebAssembly] Symbol changes #1, LLVM.
Mon, Jan 15, 8:50 AM
ncw updated the diff for D41955: [WebAssembly] Symbol changes #5: add "symbol index" concept (LLD).

Simple rebase to fix conflicts. The two problems above remain.

Mon, Jan 15, 7:25 AM
ncw updated the diff for D41954: [WebAssembly] Symbol changes #5: add "symbol index" concept (LLVM).

Simple rebase to fix conflicts.

Mon, Jan 15, 7:21 AM

Sat, Jan 13

ncw created D42028: [WebAssembly] Fix build failures due to warning.
Sat, Jan 13, 3:46 AM

Fri, Jan 12

ncw updated the diff for D41955: [WebAssembly] Symbol changes #5: add "symbol index" concept (LLD).

More updates, fixed all but two tests.

Fri, Jan 12, 4:45 PM
ncw added a comment to D40845: [WebAssembly] COMDAT: LLD support.

Thank you for reviewing and landing! And also for the "setComdat/isComdat" fix in the Clang WasmObjectWriter, I missed a trick there.

Fri, Jan 12, 2:41 PM
ncw updated the diff for D41955: [WebAssembly] Symbol changes #5: add "symbol index" concept (LLD).

Updated:

  • Some fixes after rebase
  • Most tests now passing (a couple of bugs still to iron out)
Fri, Jan 12, 10:32 AM
ncw updated the diff for D41954: [WebAssembly] Symbol changes #5: add "symbol index" concept (LLVM).

Changes:

  • Responded to Sam's feedback
  • Fixed all test failures
Fri, Jan 12, 10:10 AM
ncw accepted D41975: [WebAssembly] Don't allow functions to be named more than once.

Fine by me then!

Fri, Jan 12, 6:51 AM
ncw updated the diff for D40845: [WebAssembly] COMDAT: LLD support.

Simple rebase, to fix conflict with master after latest commits

Fri, Jan 12, 3:31 AM
ncw added a comment to D41975: [WebAssembly] Don't allow functions to be named more than once.

I've just rewritten the name handling in D41955. What I did there was to associate the name with the Function (not the Symbol) since that's conceptually what the "names" section is (the debugger's name for the function body itself).

Fri, Jan 12, 2:54 AM

Thu, Jan 11

ncw added a comment to D41954: [WebAssembly] Symbol changes #5: add "symbol index" concept (LLVM).

Thanks for the early feedback!

Thu, Jan 11, 3:24 PM
ncw created D41955: [WebAssembly] Symbol changes #5: add "symbol index" concept (LLD).
Thu, Jan 11, 10:46 AM
ncw created D41954: [WebAssembly] Symbol changes #5: add "symbol index" concept (LLVM).
Thu, Jan 11, 10:45 AM
ncw added inline comments to D41941: [WebAssembly] Change int_fast16_t to 32-bit.
Thu, Jan 11, 10:20 AM
ncw added inline comments to D41941: [WebAssembly] Change int_fast16_t to 32-bit.
Thu, Jan 11, 9:43 AM
ncw added inline comments to D41941: [WebAssembly] Change int_fast16_t to 32-bit.
Thu, Jan 11, 9:34 AM
ncw abandoned D41390: [WebAssembly] LLD: Don't write out discarded function symbols.

I'm abandoning this!

Thu, Jan 11, 4:27 AM
ncw updated the diff for D41390: [WebAssembly] LLD: Don't write out discarded function symbols.

Updated to be on top of D40845, and removed non-quite-correct handling of globals/segments.

Thu, Jan 11, 4:26 AM
ncw added inline comments to D40845: [WebAssembly] COMDAT: LLD support.
Thu, Jan 11, 4:22 AM
ncw updated the diff for D40845: [WebAssembly] COMDAT: LLD support.

Changes:

  • Disentangled from D41390
  • Changed to assume that all Functions/InputSegments are covered by Symbols. The "discardable" entities are the function and segment definitions, not the Symbols referring to them, so previously I was iterating over the Functions/Segments to make sure they were discarded. We know though that the MCWasmWriter will never output a COMDAT Function/Segment without an accompanying Symbol, so the change is safe.
Thu, Jan 11, 4:22 AM
ncw created D41941: [WebAssembly] Change int_fast16_t to 32-bit.
Thu, Jan 11, 2:32 AM

Wed, Jan 10

ncw updated the diff for D40845: [WebAssembly] COMDAT: LLD support.

Rebased as requested.

Wed, Jan 10, 6:17 AM
ncw updated the diff for D41390: [WebAssembly] LLD: Don't write out discarded function symbols.

Rebased as requested. Tests pass.

Wed, Jan 10, 6:13 AM
ncw accepted D41891: [WebAssembly] Move relocation handling to InputChunks.cpp. NFC.

Looks fine, it's just moving some code around (to a better place).

Wed, Jan 10, 1:17 AM

Tue, Jan 9

ncw added a comment to D40844: [WebAssembly] COMDAT: core LLVM support.

MC tests pass, but I've just noticed I missed one in CodeGen/WebAssembly/comdat.ll. I won't be able to fix it today, sorry..

Tue, Jan 9, 11:45 AM
ncw updated the diff for D40844: [WebAssembly] COMDAT: core LLVM support.

NB. I haven't yet re-run the tests, this is an optimistic update - it takes a looong time to compile everything, and after a rebase pretty much everything needs to recompile. Just building the tests now.

Tue, Jan 9, 11:00 AM

Sat, Dec 23

ncw added inline comments to D41510: [WebAssembly] MC: Fix for address taken aliases.
Sat, Dec 23, 4:36 AM

Fri, Dec 22

ncw accepted D41510: [WebAssembly] MC: Fix for address taken aliases.

Thanks, you're right that's nicer. I'll leave the bugzilla issue open for now until the Linking.Mr issue is closed as well.

Fri, Dec 22, 12:40 AM

Thu, Dec 21

ncw accepted D41419: [WebAssembly] Add InputChunk as common base class for InputSegment and InputFunction. NFC..

On another note, if you're in the mood for merging things, could you merge your chain of refactorings (D41315, D41419, etc)? Then other dependent changes like D41390 and Comdat can be unblocked.

Thu, Dec 21, 4:03 PM
ncw added a comment to D41472: [WebAssembly] Fix local references to weak aliases.

I agree that we are pushing the limits here of modeling the symbol table via imports and exports, so it can seem a little hacky. In the long run I agree that we may need to model the symbol table more explicitly, but I think that is a large scale change, that will take some planning. This kind of incremental fix isn't meant to block the way for more sweeping one. I actually think that modeling a weak alias as both an import and an export is quite a neat tick :) The import can then be resolved by its own export, in the absence of any strong symbol. In a sense the object both exports a version of this symbol and also depends on an external version.

Thu, Dec 21, 3:57 PM
ncw added a comment to D41510: [WebAssembly] MC: Fix for address taken aliases.

Looks like it should work. One thing I wonder about is the size of the table? It looks like we're now writing out a table entry for each relocation, which is (a tiny amount of) bloat and makes function pointers not compare equal (which is irrelevant for the object file which can't run anyway). It was neater before to be writing out the table entry only once per symbol.

Thu, Dec 21, 3:47 PM
ncw added a comment to D41472: [WebAssembly] Fix local references to weak aliases.

Ouch, hacky hacky!

Thu, Dec 21, 3:18 AM
ncw added a comment to D41460: [WebAssembly] Improve weak alias tests cases.

I haven't looked into this in detail (I'd need to hand-inspect the wasm-dis output to be sure that the obj2yaml output is actually correct). But I'm sure you've done that! Fine by me.

Thu, Dec 21, 3:07 AM
ncw added a comment to D41449: [WebAssembly] Add extra test for weak global symbols.

Since the relocation is emitted in the function body, which is tested, it should be good enough for now.

Thu, Dec 21, 2:56 AM

Wed, Dec 20

ncw updated the diff for D40845: [WebAssembly] COMDAT: LLD support.

Rebased on top of Sam's refactoring work...

Wed, Dec 20, 8:12 AM
ncw retitled D40844: [WebAssembly] COMDAT: core LLVM support from Wasm COMDAT: core LLVM support to [WebAssembly] COMDAT: core LLVM support.
Wed, Dec 20, 8:08 AM
ncw retitled D41390: [WebAssembly] LLD: Don't write out discarded function symbols from Wasm LLD: Don't write out discarded function symbols to [WebAssembly] LLD: Don't write out discarded function symbols.
Wed, Dec 20, 7:31 AM
ncw updated the diff for D41390: [WebAssembly] LLD: Don't write out discarded function symbols.

Updated:

  • Rebased to be on top of Sam's D41419
  • Added handling of global symbols, allowing InputSegments to be discarded as well if the InputSegment contains nothing else apart from the discarded Global
Wed, Dec 20, 7:21 AM
ncw updated the diff for D41449: [WebAssembly] Add extra test for weak global symbols.

Oops, rebased to master than on top of my previous patches

Wed, Dec 20, 7:17 AM
ncw created D41449: [WebAssembly] Add extra test for weak global symbols.
Wed, Dec 20, 6:50 AM
ncw added inline comments to D41315: [WebAssembly] Output functions individually.
Wed, Dec 20, 5:06 AM
ncw added inline comments to D40844: [WebAssembly] COMDAT: core LLVM support.
Wed, Dec 20, 4:50 AM
ncw updated the diff for D40844: [WebAssembly] COMDAT: core LLVM support.

Updated:

  • Responded to Sam's comments
  • Fixed a minor issue with the function indices
  • Fixed a bug in my addition to yaml2wasm
Wed, Dec 20, 4:50 AM
ncw added a comment to D41390: [WebAssembly] LLD: Don't write out discarded function symbols.

Sorry also for the lengthy responses!

Wed, Dec 20, 1:04 AM

Tue, Dec 19

ncw added inline comments to D41390: [WebAssembly] LLD: Don't write out discarded function symbols.
Tue, Dec 19, 2:07 PM
ncw added a comment to D41410: [WebAssembly] Apply data relocation in parallel. NFC..

Looks nice. My changes for pruning functions/segments rely on the relocations happening nice and late, so that we don't try and generate the index of a function that's being pruned. There changes all help with that, ensuring that we don't need the relocations to be "resolvable" for code that we're discarding.

Tue, Dec 19, 1:47 PM
ncw updated the diff for D40845: [WebAssembly] COMDAT: LLD support.

I've now split some of this out into D41390.

Tue, Dec 19, 8:54 AM
ncw updated the diff for D41390: [WebAssembly] LLD: Don't write out discarded function symbols.

Updated to make the diff a bit shorter/tidier.

Tue, Dec 19, 8:39 AM
ncw created D41390: [WebAssembly] LLD: Don't write out discarded function symbols.
Tue, Dec 19, 5:50 AM
ncw added inline comments to D41315: [WebAssembly] Output functions individually.
Tue, Dec 19, 4:07 AM
ncw abandoned D40559: Wasm entrypoint changes #2 (export entrypoint in "start" section) APPLY AFTER D40724.

Abandoning. There's not much that can be salvaged from here, it's all superseded.

Tue, Dec 19, 3:28 AM
ncw abandoned D40614: Add .init_array support to Wasm LLD.

Abandoning - but! I need to remember to keep the test files, which might still be useful.

Tue, Dec 19, 3:26 AM