US-Pacific timezone.
User Details
- User Since
- Jan 7 2013, 9:35 AM (533 w, 5 d)
Thu, Mar 30
I guess the new test covers the stub->bitcode lib dependence case. Is there a test that covers the codgen->libcall->stub case?
i.e. it seems like it would be simpler to just support processing the stubs once. or maybe more specifically, to handle them the same as other archive libraries (i.e. participate in the iterate-to-convergence phase of symbol resolution but not get special treatment for LTO?)
Do we really need to support stub libcalls (i.e. the kind that can be generated at codegen time)? These should just be compiler-rt and maybe a few core libc functions, right? Do we expect those to be implemented in JS and/or the environment?
Mon, Mar 27
Fri, Mar 24
cool, LGTM!
Thu, Mar 23
I think we can go ahead. We can always adjust emscripten's behavior as needed to synchronize with wasi-sdk
Tue, Mar 21
LGTM I guess. TBH I'm still a little on the fence about whether it's better to have fully auto-generated test expectations which are easy to update but check all the output, or more precisely targeted expectations which are more self-documenting (since they don't include output not relevant to the test at hand) and are less likely to require updating for unrelated changes...
Mon, Mar 13
When making DBG_VALUE/DBG_VALUE_INST instructions undefined
Otherwise this LGTM, but I'd still like to hear Dan's opinion if he's got one.
I guess you mean not stack_pointer but tls_base and __memory_base, right?
Fri, Mar 10
One other question about the test. Did you generate it from a C file? When I've made tests like this in the past I've usually included the C source as a comment at the top to make it easier to regenerate if needed.
Maybe we can land this as-is and see how it works in the debugger, and work on the test more if we improve the code.
Ah right. With regular codegen tests we don't have this kind of problem because the assembly output is symbolic; also some tests that test disassembly use objdump -dr which prints relocation info next to instructions that need relocations. But I don't think dwarfdump will do that.
I agree that moving this test to lld doesn't seem great either.
Thu, Mar 9
I think this feature could also be useful in non-emscripten environments, e.g. you could have a stub library for each WASI API provided by the embedder. Does wasi-sdk just use --allow-undefined for this now? Stub libraries seem nicer than that.
@sunfish WDYT, does this look useful?
Tue, Mar 7
Mon, Mar 6
I love these kinds of debugging options...
Fri, Mar 3
Mar 2 2023
- clang-format
- more cast?
Sorry about that, will fix.
And I guess I'll have to get into the habit of the pre-merge checks being useful!
Mar 1 2023
- add comment and rename hashBuf
- clean up baked-in buffer size
I think this is ready for another review, PTAL.
Feb 27 2023
Feb 21 2023
Correctly rebase against main
Rebase, create valid UUID for fast hash build ID, remove md5
Feb 10 2023
Do those figures include using wasm-opt after linking?
LGTM with a test.
Feb 3 2023
WebAssembly LGTM
Jan 31 2023
Jan 26 2023
@tlively does this test need the shuffle indices to test what it's intended to?
Jan 19 2023
Sorry, no :)
Jan 9 2023
It looks like all the feedback has been address for now, so LGTM.
We can always update the comments if we have more discussion.
I think the format should be the same for wasm64
Dec 22 2022
Yeah, we had some of this discussion with @sunfish back in 2016 as well. I just ended up disabling some of our tests and accepting reduced optimizations in D24053 and we never came up with a way to get them back at the time.
I haven't had a chance to think about this more since my earlier comment but thanks for writing this down, I'll come back to this in a couple of weeks (feel free to ping me if I don't).
Sorry, I missed your comment 2 days ago. See my comment below.
Dec 21 2022
Dec 20 2022
Dec 19 2022
Dec 16 2022
This looks good to me overall with the plan for followup. Maybe we can wait to land for a bit to see if @jmorse has any followup comments.
Dec 14 2022
Dec 13 2022
Dec 8 2022
Dec 7 2022
LGTM. TBH i'm fine with this not having a test, since it only affects our LLVM development experience and not the output. but it's good as-is too.
Dec 5 2022
Thanks for fixing this! I tested it on windows and it seems to work (with the caveat that it's not my primary platform either). I'll reland it. I'll add "reland" to the commit message but it also still has the link to this review issue, so that should be good enough for context.
Dec 2 2022
Thanks! do you need me to commit this?
The code looks good to me. Could you add a test case to llvm/test/tools/llvm-objcopy/wasm/add-section.test?
I guess it could be like the 2nd test case (that creates a file using echo and adds it, then checks the YAML payload).
Nov 29 2022
Nov 21 2022
LGTM! nice investigation too.
Nov 3 2022
Oct 4 2022
Oct 3 2022
Sep 14 2022
Sep 1 2022
I was actually thinking about something like the following:
Aug 31 2022
IIUC in other asm formats, you can just e.g. switch to the data section (in the middle of generating code for a function), emit a data constant, and then switch back and finish the function. In theory we could do that for wasm too, as long as we do actually switch back and finish the function. but I get that would make this error checking a lot more complicated, and I don't know of any cases where we would actually want to do that. do you?
Don't forget to make a build of LLVM with all the targets to make sure you get everything.
Aug 30 2022
Aug 22 2022
This broke us in emscripten as well (building with trunk clang against a recent-but-not-trunk version of libcxx). I can test the fix if you want.
This looks good to me from the wasm perspective