[PATCH] D52803: [GISel]: Add support for CSEing continuously during GISel passes

Tue Jan 15 11:16:06 PST 2019

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

In D52803#1349373 <https://reviews.llvm.org/D52803#1349373>, @kristof.beyls wrote:

> In D52803#1349005 <https://reviews.llvm.org/D52803#1349005>, @aditya_nandakumar wrote:
>
> > By default the CSE is enabled during IRTranslator and Legalizer now (disable with -enable-cse-in-legalizer/irtranslator=0).
>
>
> I naively wonder if unconditionally enabling a CSE optimization even at -O0 is going to lead to a poorer debug experience in some cases?

This is a good point, so I think that if we enable it for -O0 it should only be for constants.

> 
> 
>> I've also updated most tests (except some tests where it's not obvious what the test is doing where CSE is disabled).
> 
> If CSE should be enabled by default, I guess that we'll need to figure out what the tests are supposed to be doing so that there aren't many tests mainly testing non-default behaviour?

After some offline discussion, we've decided to not enable this at all at -O0 due to some minor compile time and code size regressions. The CSE here introduces copies as it must define a register when asked to, but at -O0 we don't do the copy elision and so compile time isn't improved, and it likely affects later passes like immediate selection. Overall, LGTM for enabling at -O1 or higher.

Repository:
  rL LLVM

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

https://reviews.llvm.org/D52803