[PATCH] D60580: [GlobalISel] Enable CSE in the IRTranslator & legalizer for -O0 with constants only

[Amara Emerson via Phabricator via llvm-commits]

aemerson marked an inline comment as done.
aemerson added inline comments.


================
Comment at: llvm/lib/CodeGen/TargetPassConfig.cpp:1228
 bool TargetPassConfig::isGISelCSEEnabled() const {
-  return getOptLevel() != CodeGenOpt::Level::None;
+  return true;
 }
----------------
aditya_nandakumar wrote:
> aemerson wrote:
> > aditya_nandakumar wrote:
> > > Maybe we should add a hook so targets can return the appropriate CSEConfigs for the opt level instead of the core passes making this decision for them?
> > Given that we're only doing constants here, I'd like all targets to be consistent as much as possible.
> The reason I mentioned adding a hook is that there's no easy way for a target to override the opcodes that are CSEd for any opt level (the pass chooses it for them) and the only option would be to modify it out of tree. Instead I was wondering something like
> 
> ```
> std::unique_ptr<CSEConfig> TargetPassConfig::getCSEConfigForOptLevel() {
>  return O0? make_unique<ConstantOnlyConfig>() : make_unique<CSEConfig>();
> }
> ```
> This would mean you wouldn't need to update both passes to check for opt level, and additionally this allows targets to override this with their own CSEConfig..
Ah, fair enough, I'll do that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60580