[PATCH] D155730: [PowerPC] Add a pass to merge all of the constant global arrays into one pool.

[Stefan Pintilie via Phabricator via llvm-commits]

stefanp added inline comments.


================
Comment at: llvm/lib/Target/PowerPC/PPCMergeStringPool.cpp:111
+
+} // namespace
+
----------------
amyk wrote:
> Please check that this is where the namespace should end.
I've moved this down. None of the functions needs to be accessed from outside of the class so there is no reason to take them outside of the namespace.


================
Comment at: llvm/test/CodeGen/PowerPC/O3-pipeline.ll:76
+; CHECK-NEXT:       FunctionPass Manager
+; CHECK-NEXT:       Dominator Tree Construction
 ; CHECK-NEXT:       Natural Loop Information
----------------
amyk wrote:
> Can we check if the Dominator Tree Construction is expected, or if it's just there because it's a module pass.
I did check and this seems to be related to the fact that this is a module pass. I did try to add `setPreservesAll` and I still got the extra passes so I'm not sure there is much that can be done about this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155730