[WebAssembly] Disable register stackification and coloring when not optimizing

[dschuff]

Summary

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.

[dschuff]

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.

[dschuff]

So it turns out that getOptLevel() isn't even hooked up unless you register an MCCodeGenInfo for the target. So I added that.

[dschuff]

• Hook up MCCodeGenInfo

[dschuff]

• accept Static as well as Default reloc model

[dschuff]

• make it report_fatal_error instead of assert

[sunfish]

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

[dschuff]

• Default to PIC and force large code model

[dschuff]

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.

[sunfish]

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

lib/Target/WebAssembly/WebAssemblyRegColoring.cpp

 #include "llvm/CodeGen/MachineRegisterInfo.h"
 #include "llvm/CodeGen/Passes.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/raw_ostream.h"
 using namespace llvm;
 
 #define DEBUG_TYPE "wasm-reg-coloring"
 
-static cl::opt<bool>
-    DisableRegColoring("disable-wasm-reg-coloring", cl::Hidden, cl::init(false),
-                       cl::desc("Disable WebAssembly register coloring"));
-
 namespace {
 class WebAssemblyRegColoring final : public MachineFunctionPass {
 public:
   static char ID; // Pass identification, replacement for typeid
   WebAssemblyRegColoring() : MachineFunctionPass(ID) {}
 
   const char *getPassName() const override {
     return "WebAssembly Register Coloring";

 }
 
 bool WebAssemblyRegColoring::runOnMachineFunction(MachineFunction &MF) {
   DEBUG({
     dbgs() << "********** Register Coloring **********\n"
            << "********** Function: " << MF.getName() << '\n';
   });
 
-  if (DisableRegColoring)
-    return false;
-
   // If there are calls to setjmp or sigsetjmp, don't perform coloring. Virtual
   // registers could be modified before the longjmp is executed, resulting in
   // the wrong value being used afterwards. (See <rdar://problem/8007500>.)
   // TODO: Does WebAssembly need to care about setjmp for register coloring?
   if (MF.exposesReturnsTwice())
     return false;
 
   MachineRegisterInfo *MRI = &MF.getRegInfo();

lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp

void WebAssemblyPassConfig::addPostRegAlloc() {
   // them.
   //
   // We use our own PrologEpilogInserter which is very slightly modified to
   // tolerate virtual registers.
   disablePass(&PrologEpilogCodeInserterID);
   // Fails with: should be run after register allocation.
   disablePass(&MachineCopyPropagationID);
 
+
+  if (getOptLevel() != CodeGenOpt::None) {
     // Mark registers as representing wasm's expression stack.
     addPass(createWebAssemblyRegStackify());
 
     // Run the register coloring pass to reduce the total number of registers.
     addPass(createWebAssemblyRegColoring());
+  }
 
   TargetPassConfig::addPostRegAlloc();
 
   // Run WebAssembly's version of the PrologEpilogInserter. Target-independent
   // PEI runs after PostRegAlloc and after ShrinkWrap. Putting it here will run
   // PEI before ShrinkWrap but otherwise in the same position in the order.
   addPass(createWebAssemblyPEI());
 }