[PATCH] D28117: [NewGVN] Merge unconditional branches before value numbering

Davide Italiano via llvm-commits llvm-commits at lists.llvm.org
Mon Dec 26 09:52:47 PST 2016


On Mon, Dec 26, 2016 at 9:47 AM, Daniel Berlin <dberlin at dberlin.org> wrote:
> I don't think NewGVN should do this, instead of a cleanup pass, unless it
> actually produces better analysis results.
>
> I'm already tempted to have us stop wasting time checking
> isInstructionTriviallyDead.
>

I have that in my todolist. In my very non-systematic tests it doesn't
actually improve things substantially.

> I know it matches what GVN does, but that doesn't seem a good reason to me,
> and i don't see how it would improve a PRE that properly does dataflow
> analysis.
> In fact, you can prove it can't :)
>

I agree, see my comment in the bug https://llvm.org/bugs/show_bug.cgi?id=31468

> (if the branch is unconditional, and can be merged, it's existence should
> provably not affect the solution to PRE's dataflow equations).
>
> Does it actually improve elimination?
>

I don't think it does, in fact I think it makes sense to just change
the test. I'm all for changing behaviour between old and newGVN when
it's needed.

Thanks!

--
Davide


More information about the llvm-commits mailing list