-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[Swift+WASM] [IRGen] disable debugger tuning and atomics on WebAssembly #24119
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
||
// WebAssembly doesn't support atomics or DWARF5 yet. | ||
if (Clang->getTargetInfo().getTriple().isOSBinFormatWasm()) { | ||
TargetOpts.DebuggerTuning = llvm::DebuggerKind::Default; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why change the debugger tuning? That controls whether the debug info is better suited for LLDB or for GDB. You want the DebugVersion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tuning for LLDB causes llvm to always generate DWARF5 accel tables: https://github.com/apple/swift-llvm/blob/8d1b92b83c0bd9735f4b8771b26ce96f8c2a3724/lib/CodeGen/AsmPrinter/DwarfDebug.cpp#L300
I'll change the comment to clarify this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
llvm has a -accel-tables option, but that isn't a great solution either. I think the best solution would be to lower the DWARF version to DWARF4 for the WASM platform.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@adrian-prantl DWARF version is already set to 4 here https://github.com/apple/swift/blob/5bef9f21187d4297253892a2220737ef222d2689/include/swift/Basic/Dwarf.h#L24 and we don't set it to 5 anywhere for Wasm, so I assume these tweaks are still needed to turn off accel tables? Or would you suggest us to use some other setting for this?
LLVM doesn't support LLDB debugger tuning for WebAssembly (since lldb tuning generates a DWARF5 accel table even when DWARF5 is disabled, and WebAssembly doesn't support DWARF5). In addition, support for atomics is currently very limited. Set LLVM target options to avoid generating DWARF5 debug data and lower atomics to regular load/stores when building for WebAssembly. This shouldn't affect existing platforms.
a133b13
to
0583894
Compare
@compnerd Updated comment and commit message to clarify why debugger options generates dwarf5 accel tables. |
Okay, so now I'm curious. Why does the emission of the DWARF accelerator tables cause problems? Not supporting the accelerator tables is one thing, which means that the data is generated but not used. Why does generating them cause any problems? |
@compnerd LLVM doesn't set the section names for dwarf5 sections, so you get a null pointer exception when it tries to create a section. Clang also runs into this issue if you pass -glldb: https://godbolt.org/z/wlJOns An alternative way to fix this is to add DWARF5 support to LLVM. I haven't tried this, since I don't know what else, in addition to adding the section names, would be required to get dwarf5 working on WebAssembly. |
I think that adding the sections for now is a good start and the right approach. I imagine that as this progresses you will find the areas that need to be fleshed out for DWARF5 support. |
@compnerd would you be able to have another look please? Is there anything that is still required for this PR to pass the review? |
@stephentyrone is this something you could review please? Could I request a review from you for any WebAssembly-related PRs or only those that are related to stdlib? |
@MaxDesiatov This is far outside my realm of expertise, sorry. |
@stephentyrone sorry for pestering, but would you be able to suggest if there's any other member of the team who worked on this area and could have a look? I would be happy to pick up this and a few other WebAssembly PRs, but not sure if there's anyone who could do the review |
Usually your best bet is to look at who's made changes here in the past. Maybe @jckarter or @aschwaighofer. |
I'm not all that familiar with wasm either, but if generating debug info in a certain way is causing LLVM to crash when targeting wasm, that seems like something to ideally fix in the LLVM wasm backend rather than hack around in Swift. |
|
||
// WebAssembly doesn't support atomics or DWARF5 yet. | ||
if (Clang->getTargetInfo().getTriple().isOSBinFormatWasm()) { | ||
TargetOpts.DebuggerTuning = llvm::DebuggerKind::Default; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
llvm has a -accel-tables option, but that isn't a great solution either. I think the best solution would be to lower the DWARF version to DWARF4 for the WASM platform.
Ah I see, you're right. I think it would be more general to address this in LLVM though. Either by special-casing WASM in that function based on the triple, or even better by implementing the missing support. |
Closing as outdated |
LLVM doesn't support generating DWARF5 debugging data for WebAssembly, and support for atomics is currently very limited.
Set LLVM target options to avoid generating DWARF5 debug data and lower atomics to regular load/stores when building for WebAssembly.
This shouldn't affect existing platforms.
@ddunbar @MaxDesiatov This should be helpful for SR-9307.