These passes are optimizations, and should be disabled when not
optimizing.
Also remove the command line flag for disabling register coloring;
running llc with -O0 should now be useful for debugging, so it's not
necessary.
Details
Diff Detail
- Repository
- rL LLVM
Event Timeline
Arguably we should make some (a lot?) of our tests run without optimization since the stackifier is orthogonal to what most of them test and it munges the output in unrelated ways and will cause churn when the stackification code changes. That doesn't seem as common in backend tests as maybe it should be.
So it turns out that getOptLevel() isn't even hooked up unless you register an MCCodeGenInfo for the target. So I added that.
I assume the purpose here is debugging compiler output, however -O0 is generally meant for debugging source code. For example, LLVM's normal CodeGen path does do (limited) register allocation at -O0. User variables are generally stored in allocas, so they aren't affected by register allocation. Consequently, disabling register stackifying and coloring at -O0 isn't obviously the right thing to do.
Some of our tests do use -disable-block-placement, for example, to reduce uninteresting optimizer churn when it's a problem. Using -disable-wasm-register-coloring seems like it would be useful in some cases as well. A flag to disable register stackifying could be similarly useful.
lib/Target/WebAssembly/MCTargetDesc/WebAssemblyMCTargetDesc.cpp | ||
---|---|---|
48 ↗ | (On Diff #48159) | I agree with being conservative for such things in general, however WebAssembly code is always PIC, so we already do support all the reloc modes. Accepting all the modes is useful because user build scripts sometimes add -fPIC in target-insensitive ways, for example. I propose we unconditionally override the mode to PIC_, as Darwin does on newer architectures. |
50 ↗ | (On Diff #48159) | WebAssembly doesn't have offset restrictions, so it technically supports the "large" model by default. I propose we map Default and JITDefault to Large, and then accept only Large (rejecting Small, Medium, and Kernel in case we want to make those mean something some day). |
That's a good point about user code; I don't really feel strongly about it. Although, register stackification does reorder code, which does affect debugging; so maybe we'd still want to disable that. Also presumably compilation speed is the other factor. Coloring presumably isn't linear, so maybe we'd want to disable that for speed reasons.
lib/Target/WebAssembly/MCTargetDesc/WebAssemblyMCTargetDesc.cpp | ||
---|---|---|
48 ↗ | (On Diff #48159) | Good point, done. |
I expect eventually we'll want to do *some* coloring and stackification at -O0, though it's a good point that stackification can reorder things in ways that make debugging harder. Perhaps at -O0 stackification can just stackify things that don't require reordering. In any case, we can work that out later, and this patch lgtm for now.
lib/Target/WebAssembly/MCTargetDesc/WebAssemblyMCTargetDesc.cpp | ||
---|---|---|
47 ↗ | (On Diff #48198) | Looks good. We should handle DefaultJIT as Large too. Of course, we don't have full JIT support yet, but that's no reason for this code not to be ready for when we do :-). |