nice small reproducer too.

Looks straightforward, thanks for tracking this down.

I think this makes sense.

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?

Got it, this makes sense to me too. And since it can be turned off, I'm not too worried about annoying users.

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.

(do you want me to land this?)

In D102589#2763870, @jmorse wrote:

Until recently, variable locations that could not be described became "DBG_VALUE $noreg" instructions. With some recent patches from @StephenTozer , some of those locations can now be described with DBG_VALUE_LIST instructions. If I understand this patch correctly, for WebAssembly those DBG_VALUE_LIST instructions transition back to being "DBG_VALUE $noreg". There should be no regression in variable location coverage: instead, WebAssembly does not yet benefit from the additional locations now being recovered.

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).

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).

thanks, still LGTM

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

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)

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?)

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

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.

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.

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.

Otherwise the code looks good though

Oh also, I think --export-if-defined is an OK name.

Pre-existing, but what does --export plus --allow-undefined do?

LGTM assuming the test is what you intended.

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.

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.

otherwise LGTM

Would it make sense to also run e.g. llvm/test/MC/WebAssembly/debug-localvar.ll with split-dwarf?

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.

fix diff

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.