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

[Philip Reames via Phabricator via llvm-commits]

reames requested changes to this revision.
reames added inline comments.
This revision now requires changes to proceed.


================
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"));
----------------
Please add the explicit cl::init(false) rather than relying on default initialization.


================
Comment at: llvm/lib/Target/RISCV/RISCVTargetMachine.cpp:213
+
+  if (EnableGlobalMerge == cl::BOU_TRUE) {
+    addPass(createGlobalMergePass(TM, /* MaxOffset */ 2047,
----------------
Can't this just be if (EnableGlobalMerge)?


================
Comment at: llvm/test/CodeGen/RISCV/global-merge-offset.ll:2
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: sed 's/ArrSize/100/g' %s | llc -mtriple=riscv32 -riscv-enable-global-merge \
+; RUN:   -verify-machineinstrs | FileCheck %s
----------------
The use of sed here is simply overly complicating things.

Please add a second test to this file which uses a separate global with an alternate size.  


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

https://reviews.llvm.org/D130481