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

[Philip Reames via Phabricator via llvm-commits]

reames accepted this revision.
reames added a comment.
This revision is now accepted and ready to land.

LGTM

On the test question, decided I didn't really care.  I think we probably could construct this without sed, but the marginal improvement in readability isn't worth further delay.  I might take a shot at a follow up patch on that issue specifically, or I might not.



================
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"));
----------------
craig.topper wrote:
> 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.
I'm not familiar with usage of boolOrDefault.  You can ignore my comment here if you believe boolOrDefault is a reasonable choice here.  I'll defer to your judgement.


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

https://reviews.llvm.org/D130481