- User Since
- Jan 7 2013, 9:35 AM (441 w, 8 h)
Thu, Jun 17
Tue, Jun 1
Wed, May 26
nice small reproducer too.
Looks straightforward, thanks for tracking this down.
Mon, May 24
I think this makes sense.
May 21 2021
Ah, got it. So is it the case that we have e.g. a single .tdata section that might be separate now, but just 1 or 2 and not scaling with the size of the program.
Mostly I just want to make sure we don't slow down loading too much, especially in the dylib case where it will already be slower than the non-library case.
Does avoiding combing data segments in shared-memory mode mean that there will have to be a lot more passive segment initializations run at load time, instead of just one before?
May 20 2021
Got it, this makes sense to me too. And since it can be turned off, I'm not too worried about annoying users.
May 19 2021
BTW Is there a way to disable this warning?
Since IIUC this could cause code that was not warning (because it used -fno-exceptions or used emscripten's default of -fignore-exceptions) to now start warning, sometimes that makes users who use -Werror unhappy.
May 18 2021
May 17 2021
(do you want me to land this?)
If I'm understanding this correctly, we only create a WebAssemblyDebugValueManager object when we actually stackify a reg and move the def, correct?
I guess that still ends up invalidating a lot of debug info, since we always run explicit locals.
In the code you've seen, does the debug info all or mostly consist of DBG_VALUE_LISTs now, or is it still mostly DBG_VALUE, or a mix?
I'm just trying to get a sense for how much of a degradation in debug info quality we have now.
(Not that we shouldn't do this; clearly this is better than the loop-explosion we have now. I'm just trying to get a sense for how urgent the "real" fix is).
May 12 2021
May 11 2021
I'm not sure I understand what the description means, in particular how null-termination is related to the symbol extending past the end of the segment. (more generally I guess I don't understand why we want to allow symbols to extend past the end).
May 10 2021
thanks, still LGTM
May 7 2021
May 4 2021
Thanks, interesting about DbgEntityHistoryCalculator, I saw it but thought that it was just part of the way the translation from dbg_values to DWARF/other debuginfo was done in the platform-independent code. And to be clear, our debug info generation appears to actually be working correctly; it's only the MachineVerifier that is complaining.
I'll take a closer look at DbgEntityHistoryCalculator
May 3 2021
Is throw also a terminator?
So this change means that unresolved symbols will turn into imports when linking?
I think this is pretty much ok now, other than that I would like to make it more clear exactly which parts are more-or-less directly copied from ELF; that way if we need to understand more in the future, we know where to look (and also if there are changes to ELF in the future we would know to look at those to see if they make sense for us too)
Apr 30 2021
I think all of emscripten's use of the table is still via the JS API (e.g. src/runtime_functions.js) so this probably won't break emscripten. It does seem likely that nobody is using these instructions via assembly.
Is this the only change needed? Do we have no tests of these asm instructions anywhere in LLVM at all?
I think I'm generally fine with keeping things that are direct copies from ELF, although it would be nice if it were even easier to tell which pieces those are (there is one comment in a header; is it just that function though?)
Apr 29 2021
looks pretty good overall, a lot of this is making sure I understand everything
Still looking over this, but had a question more up front
Apr 27 2021
Apr 26 2021
LGTM modulo MaskRay's comment about the help text. Also I think @sunfish had a question earlier today that we didn't get to talk about.
Apr 23 2021
Apr 22 2021
Would it make sense for you to to upstream an LLVM target such as le32-halide? (Or perhaps even arm32-halide or some other?) Then you'd actually have more control over your own codegen, datalayout, etc.
Apr 21 2021
Thanks. I had heard in the past that there were some other folks who had used le32/le64 as a "generic" target (in fact that's why it's named so generically, rather than being called "pnacl" or similar) but I haven't heard of anything recently, and as you can see nobody has upstreamed any support for other OS or target specializations or asked to collaborate on it. Practically speaking even a target that wants fairly generic bitcode would probably want its own triple, so unless this removal captures someone's attention who wants to keep maintaining it, this should be fine to remove.
This looks good. Since you've created new build components, don't forget to test the LLVM/clang build and make sure it works with static libraries and with the LLVM dylib before you land.
Apr 20 2021
Apr 7 2021
Otherwise the code looks good though
Apr 5 2021
Oh also, I think --export-if-defined is an OK name.
Pre-existing, but what does --export plus --allow-undefined do?
Mar 31 2021
Mar 30 2021
Mar 29 2021
Mar 12 2021
Mar 4 2021
LGTM assuming the test is what you intended.
Mar 3 2021
I agree this is a good approach, and I also like that it's smaller and simpler. In the future we can revisit whether following this particular Itanium convention buys us anything useful or not.
Mar 1 2021
Feb 26 2021
LGTM. since the data structure is a SmallPtrSet now, it probably makes sense to update the description and maybe the var names to reflect that.
Would it make sense to also run e.g. llvm/test/MC/WebAssembly/debug-localvar.ll with split-dwarf?
Feb 23 2021
The description is great, and I think this change is the right thing to do for now. Once things stabilize, it might eventually make sense to think some more about how we might simplify the sorting algorithm (as you discussed in the description). And you're right that we'd want to have better performance/benchmarking in place to inform that.
Feb 22 2021
Feb 18 2021
Feb 17 2021
Update existing test instead of using new one
Ah, you are right, I initially thought that we needed a new test because none of ours failed when their patch went in. But the reason for that was that the output is all assembly rather than object files.
I added --implicit-check-not to exception.ll and that does catch the error.