[PATCH] D130481: [RISCV] Add the GlobalMerge pass (disabled by default)

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVTargetMachine.cpp:47
+static cl::opt<cl::boolOrDefault>
+    EnableGlobalMerge("riscv-enable-global-merge", cl::Hidden,
+                      cl::desc("Enable the global merge pass"));
----------------
reames wrote:
> Please add the explicit cl::init(false) rather than relying on default initialization.
It's not relying on default initialization. It's a `cl::boolOrDefault` option not a `bool` option. This due to the usage in the original patch where it has 3 behaviors.


================
Comment at: llvm/lib/Target/RISCV/RISCVTargetMachine.cpp:213
+
+  if (EnableGlobalMerge == cl::BOU_TRUE) {
+    addPass(createGlobalMergePass(TM, /* MaxOffset */ 2047,
----------------
luismarques wrote:
> reames wrote:
> > Can't this just be if (EnableGlobalMerge)?
> This seems to be used in various other places.
It's not a `bool` option it's `boolOrDefault` option so it has 3 values not 2.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130481/new/

https://reviews.llvm.org/D130481