[PATCH] D39011: [SimplifyCFG] try harder to forward switch condition to phi (PR34471)

Sanjay Patel via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sun Oct 22 10:46:45 PDT 2017


spatel added a comment.

In https://reviews.llvm.org/D39011#903093, @dberlin wrote:

> Hey Sanjay,
>  GVN/NewGVN/et al will undo this transform, and propagate constants. In fact, i expect pretty much any pass that can, will.
>
> This definitely belongs in the late simplification category at best.
>
> More than that, i'm concerned this is the wrong fix.
>
> Why is the right answer  to undo the transform (which will cause fights between passes)  in simplifycfg instead of improving the codegen to recognize the issue?
>
> It seems the same code you are using to detect and undo the transforrm in simplification could be used to codegen it better.


I didn't realize other passes were undoing this transform. I thought I was just improving the existing canonical form favored by SimplifyCFG. Ie, the existing code in ForwardSwitchConditionToPHI() is already fighting with the other passes, correct?

So I'm certainly open to suggestions on how to proceed. Should I revert this change or delay everything in ForwardSwitchConditionToPHI() to -latesimplifycfg (note: I proposed a refinement of that in https://reviews.llvm.org/D38631)? And then follow that with moving everything to codegen (I haven't actually looked at switches in codegen yet, so that's definitely a follow-up step if I'm doing it).


Repository:
  rL LLVM

https://reviews.llvm.org/D39011





More information about the llvm-commits mailing list