[PATCH] D59139: [CodeGenPrepare] Fix ModifiedDT flag in optimizeSelectInst

Wed Mar 27 09:03:27 PDT 2019

tejohnson added a comment.

In D59139#1444332 <https://reviews.llvm.org/D59139#1444332>, @spatel wrote:

> If #1 is a one-liner that doesn't change any existing tests, that sounds like a good choice.

Basically, yes (I'd also remove the ModifiedDT parameter to that routine).

> #2 / #3 also sound like good options. IMO, the fact that we have or need to make that kind of structural change points out that CGP itself has gotten too big. According to its lead comment, this is supposed to be temporary spot for hacks prohibited by SDAG (ie, once we switch to global-isel, CGP should go away)...but CGP has become several independent passes in 1 file. The bigger change to correct these kinds of problems would be to split things into multiple passes.

Yeah it seems that the structure is has gotten really unwieldy and haphazard (i.e. when the function walk needs to be completely restarted and which parts need to be iterative isn't clear to me).

I will go ahead and send a patch for #1 for now since it is very simple and addresses this regression specifically, by undoing the iteration change provoked by this fix.

Repository:
  rL LLVM

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

https://reviews.llvm.org/D59139