[PATCH] D22551: CodeGen: If Convert blocks that would form a diamond when tail-merged.

[Kyle Butt via llvm-commits]

iteratee added a comment.

In https://reviews.llvm.org/D22551#490028, @davidxl wrote:

> I am torn about this change. While this looks like a useful thing to do, I suspect this is not the right way to approach the problem.
>
> The example actually confirms the fact the pre-layout tailmerging is a good normalization/enabler pass for later optimizations. This is the reason why it should be run with lower threshold enabling as much optimization as possible, and later let TailDup to undo those that do not bring benefit and to improve layout.


I couldn't find any of the original commits about tail merging that refer to normalization. It's always about reducing code size. Also, it's unusual for a canonicalization pass to have a threshold.

> Another question is that whether this patch can handle more cases where (tailMerge + ifcvt) can not handle. If not, it seems to me the patch seems to have duplicated logics (e.g, counting dups) in tailMerge which is not the right approach.


Two things:

1. The code has to count duplicates anyway. Look at how IfConvertDiamond is written.
2. There is a precedent for teaching optimization passes to look deeper, even if there is a canonicalization pass that would prevent the need.

> Is this patch required to enable your tailDup enhancement patch? I don't think this one is essential for it.   We can probably focus on getting your tailDup patch in first (it is very close to get -- probably just to make your latest tailMerge tuning  to be enabled only in post layout mode?)


Not specifically. If tailMerge is made less aggressive only during layout, then this is not necessary.


https://reviews.llvm.org/D22551