Skip to content
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

add: hidden option to disable slow wasm pass #67715

Merged
merged 1 commit into from
Oct 18, 2023
Merged

Conversation

ajbt200128
Copy link
Contributor

Currently for any wasm target, llvm will make a pass that removes irreducible control flow. (See here). This can result in O(NumBlocks * NumNestedLoops * NumIrreducibleLoops + NumLoops * NumLoops) build time, which has resulted in exceedingly long build times when testing. This PR introduces a hidden flag to skip this pass, which brings some of our build times down from 30 minutes to ~6 seconds.

@ajbt200128
Copy link
Contributor Author

@sbc100 Hey! it looks like you sometimes review wasm stuff, would love if you could take a look at this quick PR :)

@sbc100
Copy link
Collaborator

sbc100 commented Oct 3, 2023

What happens if the program contains irreducible control flow? Will the compiler crash or fail somewhere further down the line?

@sunfishcode @dschuff @kripken

@kripken
Copy link
Member

kripken commented Oct 3, 2023

I haven't worked on this recently, so it might have changed, but back then it was not optional: later passes assumed control flow was reducible.

If that is still the case, another option here might be to see if the LLVM pass that removes irreducible control flow is faster than this (I expect it would be, since this pass works hard to find optimal shapes for wasm).

@ajbt200128
Copy link
Contributor Author

In practice LLVM doesn't crash, but I haven't tested extensively. That was part of my motivation of keeping the flag hidden.

@ajbt200128
Copy link
Contributor Author

I can investigate using the LLVM pass if that's preferable, but that feels more heavy handed to me so I'd be hesitant to. My use case for this flag is speeding up development builds, nothing production.

@ajbt200128
Copy link
Contributor Author

@kripken what else needs to be done for this PR to merge?

@kripken
Copy link
Member

kripken commented Oct 18, 2023

@ajbt200128 I am actually not a code owner here. It would be up to the other people cc'd.

@ajbt200128
Copy link
Contributor Author

@sbc100 Are you the codeowner?

@llvmbot
Copy link
Collaborator

llvmbot commented Oct 18, 2023

@llvm/pr-subscribers-backend-webassembly

Author: Austin Theriault (ajbt200128)

Changes

Currently for any wasm target, llvm will make a pass that removes irreducible control flow. (See here). This can result in O(NumBlocks * NumNestedLoops * NumIrreducibleLoops + NumLoops * NumLoops) build time, which has resulted in exceedingly long build times when testing. This PR introduces a hidden flag to skip this pass, which brings some of our build times down from 30 minutes to ~6 seconds.


Full diff: https://github.com/llvm/llvm-project/pull/67715.diff

1 Files Affected:

  • (modified) llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp (+8-1)
diff --git a/llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp b/llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp
index 10464e8f52e5612..22d8210b8ded474 100644
--- a/llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp
+++ b/llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp
@@ -48,6 +48,12 @@ static cl::opt<bool> WasmDisableExplicitLocals(
              " instruction output for test purposes only."),
     cl::init(false));
 
+static cl::opt<bool> WasmDisableFixIrreducibleControlFlowPass(
+    "wasm-disable-fix-irreducible-control-flow-pass", cl::Hidden,
+    cl::desc("webassembly: disables the fix "
+             " irreducible control flow optimization pass"),
+    cl::init(false));
+
 extern "C" LLVM_EXTERNAL_VISIBILITY void LLVMInitializeWebAssemblyTarget() {
   // Register the target.
   RegisterTargetMachine<WebAssemblyTargetMachine> X(
@@ -537,7 +543,8 @@ void WebAssemblyPassConfig::addPreEmitPass() {
   addPass(createWebAssemblyNullifyDebugValueLists());
 
   // Eliminate multiple-entry loops.
-  addPass(createWebAssemblyFixIrreducibleControlFlow());
+  if (!WasmDisableFixIrreducibleControlFlowPass)
+    addPass(createWebAssemblyFixIrreducibleControlFlow());
 
   // Do various transformations for exception handling.
   // Every CFG-changing optimizations should come before this.

@sunfishcode
Copy link
Member

Ideally we should fix the pass to not be so pathologically slow. If your code compiles successfully with the pass disabled, that probably means your code has no multiple-entry loops, which is probably a very common case, and it seems like the pass ought to be able to quickly detect and exit without doing any extra work.

However, until that's fixed, I'm ok merging this patch adding a hidden flag.

@sunfishcode sunfishcode merged commit 85b8958 into llvm:main Oct 18, 2023
4 checks passed
@tpetr
Copy link

tpetr commented Oct 18, 2023

Thanks for the merge! This makes our build so much faster.

Feel free to let me or @ajbt200128 know if you’d like more info on how to reproduce the issue.

@sbc100
Copy link
Collaborator

sbc100 commented Oct 19, 2023

@sunfishcode do you what would happen if a "multiple-entry loop" was added to codebase that uses this option? Would it fail in a useful way?

tpetr added a commit to semgrep/semgrep that referenced this pull request Jan 12, 2024
`emscripten/emsdk:3.1.51` pulls in a new enough version of llvm which
contains llvm/llvm-project#67715. Let's start
using it and save some time!

@ajbt200128 are there any languages I'm forgetting here? Or should we
blanket apply `-wasm-disable-fix-irreducible-control-flow-pass` ?
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants