Remove EnableEarlyCSEMemSSA option

[echristo]

Summary

This has been turned on for about a year and a half now and removing it helps simplify the pass structure (though not a lot).

I didn't see much of a review either way so I don't even know if people care or want it on by default as this is literally the only place that has it on by default and it appears to have been used for GVN Hoist which is still off by default.

Thoughts? Conditionalize it on GVN Hoist? Something else?

[hfinkel]

Is this still useful for debugging problems with MemSSA and GVNHoist? Is this mirrored in the new pass manager?

[george.burgess.iv]

I can't find any users of this flag, so I'm in favor of s/EnableEarlyCSEMemSSA/true/ if that simplifies things.

Is this still useful for debugging problems with MemSSA and GVNHoist?

If we just have to tweak this one place, IMO it's sufficiently straightforward to just manually flip between true/false in code. I've personally never used it to debug anything, though I can only speak for myself.

[echristo]

In D60747#1469517, @george.burgess.iv wrote:

I can't find any users of this flag, so I'm in favor of s/EnableEarlyCSEMemSSA/true/ if that simplifies things.

Is this still useful for debugging problems with MemSSA and GVNHoist?

If we just have to tweak this one place, IMO it's sufficiently straightforward to just manually flip between true/false in code. I've personally never used it to debug anything, though I can only speak for myself.

That was pretty much my thought. As far as the new pass manager - the flag isn't being used anywhere else other than this location :)

[echristo]

In D60747#1469518, @echristo wrote:

In D60747#1469517, @george.burgess.iv wrote:

I can't find any users of this flag, so I'm in favor of s/EnableEarlyCSEMemSSA/true/ if that simplifies things.

Is this still useful for debugging problems with MemSSA and GVNHoist?

If we just have to tweak this one place, IMO it's sufficiently straightforward to just manually flip between true/false in code. I've personally never used it to debug anything, though I can only speak for myself.

That was pretty much my thought. As far as the new pass manager - the flag isn't being used anywhere else other than this location :)

Except there's a subtly different one there. I'll also remove that.

-eric

[echristo]

Update for the new pass manager as well. :)

[echristo]

This sounds like people are fine with me doing this...

[george.burgess.iv]

This sounds like people are fine with me doing this...

Agreed.

Please wait until tomorrow before committing in case anyone wants to voice last-second concerns.

Thanks again!

llvm/trunk/lib/Passes/PassBuilder.cpp

RunPartialInlining("enable-npm-partial-inlining", cl::init(false),
                        cl::Hidden, cl::ZeroOrMore,
                        cl::desc("Run Partial inlinining pass"));
 
 static cl::opt<bool>
     RunNewGVN("enable-npm-newgvn", cl::init(false),
               cl::Hidden, cl::ZeroOrMore,
               cl::desc("Run NewGVN instead of GVN"));
 
-static cl::opt<bool> EnableEarlyCSEMemSSA(
-    "enable-npm-earlycse-memssa", cl::init(true), cl::Hidden,
-    cl::desc("Enable the EarlyCSE w/ MemorySSA pass for the new PM (default = on)"));
-
 static cl::opt<bool> EnableGVNHoist(
     "enable-npm-gvn-hoist", cl::init(false), cl::Hidden,
     cl::desc("Enable the GVN hoisting pass for the new PM (default = off)"));
 
 static cl::opt<bool> EnableGVNSink(
     "enable-npm-gvn-sink", cl::init(false), cl::Hidden,
     cl::desc("Enable the GVN hoisting pass for the new PM (default = off)"));
 
PassBuilder::buildFunctionSimplificationPipeline(OptimizationLevel Level,
   assert(Level != O0 && "Must request optimizations!");
   FunctionPassManager FPM(DebugLogging);
 
   // Form SSA out of local memory accesses after breaking apart aggregates into
   // scalars.
   FPM.addPass(SROA());
 
   // Catch trivial redundancies
-  FPM.addPass(EarlyCSEPass(EnableEarlyCSEMemSSA));
+  FPM.addPass(EarlyCSEPass(true /* Enable mem-ssa. */));
 
   // Hoisting of scalars and load expressions.
   if (EnableGVNHoist)
     FPM.addPass(GVNHoistPass());
 
   // Global value numbering based sinking.
   if (EnableGVNSink) {
     FPM.addPass(GVNSinkPass());

llvm/trunk/lib/Transforms/IPO/PassManagerBuilder.cpp

static cl::opt<bool>
     DisablePreInliner("disable-preinline", cl::init(false), cl::Hidden,
                       cl::desc("Disable pre-instrumentation inliner"));
 
 static cl::opt<int> PreInlineThreshold(
     "preinline-threshold", cl::Hidden, cl::init(75), cl::ZeroOrMore,
     cl::desc("Control the amount of inlining in pre-instrumentation inliner "
              "(default = 75)"));
 
-static cl::opt<bool> EnableEarlyCSEMemSSA(
-    "enable-earlycse-memssa", cl::init(true), cl::Hidden,
-    cl::desc("Enable the EarlyCSE w/ MemorySSA pass (default = on)"));
-
 static cl::opt<bool> EnableGVNHoist(
     "enable-gvn-hoist", cl::init(false), cl::Hidden,
     cl::desc("Enable the GVN hoisting pass (default = off)"));
 
 static cl::opt<bool>
     DisableLibCallsShrinkWrap("disable-libcalls-shrinkwrap", cl::init(false),
                               cl::Hidden,
                               cl::desc("Disable shrink-wrap library calls"));
if (OptLevel > 0 && !IsCS)
     MPM.add(
         createPGOIndirectCallPromotionLegacyPass(false, !PGOSampleUse.empty()));
 }
 void PassManagerBuilder::addFunctionSimplificationPasses(
     legacy::PassManagerBase &MPM) {
   // Start of function pass.
   // Break up aggregate allocas, using SSAUpdater.
   MPM.add(createSROAPass());
-  MPM.add(createEarlyCSEPass(EnableEarlyCSEMemSSA)); // Catch trivial redundancies
+  MPM.add(createEarlyCSEPass(true /* Enable mem-ssa. */)); // Catch trivial redundancies
   if (EnableGVNHoist)
     MPM.add(createGVNHoistPass());
   if (EnableGVNSink) {
     MPM.add(createGVNSinkPass());
     MPM.add(createCFGSimplificationPass());
   }
 
   // Speculative execution if the target has divergent branches; otherwise nop.