No I mean alloca as in the LLVM instruction. Just having an address-taken local in C is enough to cause an alloca in the IR.

(FTR, the failure mode is the same assertion in MFI::getFrameBaseLocal() that we saw before)

the IR to reproduce this is trivial but i'm still trying to reduce the debug info in the test. Basically this:

Thanks!

I think the way you have it written here, it means that *all* swiftcc functions will end up with the extra parameters at the wasm level, and direct calls will also have the extra arguments. I think the test should also cover that (not just indirect calls), and I think the calling convention should be documented since it is basically the core of the swift ABI (even if you don't want to consider it stable yet). So at minimum there should be more explanation in the commit message and there should probably be a doc about the Swift ABI in https://github.com/WebAssembly/tool-conventions/ like there is for C.

Thanks; please also update the commit message to match your comment update.

the diff context looks fine to me now, FWIW

I assume there's some kind of ABI doc that says how swiftself and swifterror are supposed to work and be lowered, is there one other than just the langref?

can you please upload this with full context (e.g. diff -U 999 or just use arcanist)?

Yeah, let's just go ahead and land this. I think once this rolls we can enable DWARF with -g by default and start to get more feedback.

Hm, that's too bad. In theory it seems like it ought to be possible to just remove all the debug info from the IR that's attached to functions (i.e. all the debug.declare/debug.values) and the debug metadata that's associated with all functions other than this one, since this is a completely function-local transform that really only depends on there being any CU information at all.
I've also sometimes had good luck with c-reduce to try to reduce it at the C level rather than the IR level. It might be worth a try.
Or maybe we've let this go long enough, and we could just say that we'll have coverage if we do any continuous builds of a reasonable test suite with debug info?

It's probably worth an attempt, and that sounds like the right approach. A crash bug like this is often not too hard to reduce with bugpoint; i.e. you'd start with clang using -emit-llvm to get the IR, and then let bugpoint reduce the IR.

Yeah, this definitely looks like the likely fix. It's the spot where a local gets assigned that doesn't go through getLocalId. It seems to make sense to still allow this merging, as it would reduce the total number of locals. I guess it does mean that the frame base info would be incorrect when that register has the argument's value rather than the frame base's value, but presumably that would be ok, since the frame base should be live anytime a variable on the stack is live.

Yes, that's really all I'm after here.
While I've got your attention, do you have any advice about best practices for debug info tests?
It seems like in this case kind of the minimal thing would be to take an existing small debug info test (which in many cases are clang-generated), switch the debug info version metadata tag to 5 and run it through llc. Or is there some simpler thing?

LGTM assuming removing this is what we want given D74531

Don't remove all custom sections with --strip-ally

Updated, please take a look.

rebase, add comments to test expectations

FWIW this LGTM, especially if this is the same approach used in the other backends.

fixed in rGf5f70d1c8

My bad, sorry about that.

Fixed the nit and added the test for dump-section and remove-section. The clang-format bot seems very confused, but I ran clang-format locally.

• Merge branch 'master' into objcopy-add-remove
• Address comment, add test for dump+remove

The langref says (https://llvm.org/docs/LangRef.html#call-instruction) that the tail call marker implies that the callee does not access allocas from the caller. So it seems like it *should* mean that the backend can depend on this property (that you're checking for here). It also means that the frontend should guarantee it as best it knows how, and optimizations should not introduce it (or remove the attribute if they do?). There are lots of ways to sneak pointers into places (aliasing, going through memory, etc etc) so I'd expect the check in this CL to be brittle.

It doesn't look like there's anything wasm-specific here. Surely this also inhibits tail calling in other backends too? Are frontends suppost to avoid putting 'tail call' in the IR in this case?

• address review comments, clang-format

Rebased against master, and addressed all the comments; please take a look.

• comments, and fix file errors
• dump section before removal

• Merge branch 'master' into objcopy-add-remove
• Update for fixes in previous CL

Relanded with a fix as rGf2af0607000c
I don't have a BE machine to test on, but I'm *pretty* sure it works :)

I won't have time to look at it today; i'll just revert and take a look tomorrow.

I'll go ahead and land this to reduce the number of cases where we crash; but I'd be interested in hearing if @yurydelendik has found any cases that were asserting before that this fixes, or if any of the wasm subscribers have other opinions.

• clang-format

How about just isa<WasmObjectFile>(&O) instead?

• * When stackifying the frame base, unset frameBaseVreg

Thanks! sbc and aardappel are the relevant wasm experts here.

• remove period, use doxygen comments

• remove stray empty line

• Update test CHECKs to account for removing the blank line (turns out the code actually works :)

• Fix clang-tidy warning

• review comments

Thanks!

• Rename generateSectionHeader, clarify comments

• fix clang-tidy, add comment, fix whitespace

• fix newline

• clang-format

• Add test and Fix WebAssemblyAsmParser to emit dwarf for all text sections

• Rename generateSectionHeader, clarify comments

• remove debug print

@sbc100 I haven't added any tests yet but does this look like a reasonable use of the various Symbol/Ref types?

Hm, the bot seems to be using the wrong version of the patch.