[PATCH] D45916: Enable MachineOutliner by default under -Oz for AArch64

[Eli Friedman via Phabricator via llvm-commits]

efriedma added inline comments.


================
Comment at: lib/CodeGen/TargetPassConfig.cpp:120
+    cl::init(false),
+    cl::desc("Disable machine outliner"));
 // Enable or disable FastISel. Both options are needed, because
----------------
paquette wrote:
> efriedma wrote:
> > Instead of having two flags, can you just check getNumOccurrences()?
> "enable-machine-outliner" implies "run it on everything, even things the target doesn't want by default". This isn't actually symmetric with "disable-machine-outliner", which says "don't run it on anything at all". I think "enable-machine-outliner" should be renamed to something that better describes the intended behaviour. I'll do that in a follow-up patch.
In other places, we sometimes do something like "true is always, false is never, unspecified is the default".  But I guess this isn't really the same thing.

Could you rename the flags to be more clear?  (-disable-machine-outliner-pass vs. -machine-outliner-all-functions).


================
Comment at: lib/CodeGen/TargetPassConfig.cpp:904
 
-  if (EnableMachineOutliner)
+  if (!DisableMachineOutliner)
     addPass(createMachineOutlinerPass());
----------------
We probably need to check `getOptLevel() != CodeGenOpt::None` here.


https://reviews.llvm.org/D45916