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

Tue Jan 8 07:57:48 PST 2019

aditya_nandakumar added a comment.

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?

Yes this is definitely an important question. I only enabled them by default because I wanted to test that CSE works with the pipeline. I can definitely disable CSE for O0 for each of the passes and add some more tests (besides making the unit test more comprehensive) that make sure CSE works in pipeline.

> 
> 
>> 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?

I updated most of the tests that just run a single GISel pass. I was hesitant to update the tests that take it all the way to assembly (that were not auto generated).

Repository:
  rL LLVM

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

https://reviews.llvm.org/D52803