[PATCH] D17339: [SystemZ] Implement conditional returns.

[Ulrich Weigand via llvm-commits]

uweigand added a comment.

Your code changes look good in general.   However, I agree that some of resulting transformations are less than optimal.

> The if conversion happens before fused compares and BRCT are generated, thus winning over them. Could be OK for the fused compares, I suppose, but BRCT should be better than cond return. I suppose BRCT generation could be changed to cover that case, thoiugh. I've added junk instructions before return in the CIJ* and BRCT testcases to make sure they are still emitted.


For the fused compares, I think we should be able to get the best of both worlds by first generating the conditional return, and then fusing *that* with a compare into one of the C(G)RB or C(G)IB instructions (which LLVM currently doesn't use at all).

For BRCT, I suspect the problem is a special case of the strange loop transformation you mention later.  If the backwards branch of the loop weren't modified by IfConversion, then the BRCT should still be detected.

Now as to that loop scenario, if I understand it correctly, it seems IfConversion recognizes the backwards branch of a loop at the very end of the function as an instance of the (inverted) ICSimple pattern.  This seems questionable.   I'm wondering whether this can be fixed in common code somehow (but I'm not sure if there's loop structure information available at this point in the process).

Failing that, I guess the backend's isProfitableToDupForIfCvt callback might try to detect that scenario via a low BranchProbability parameter being passed in (assuming that common code assigns a high probability to the backwards branch of a loop by default).


http://reviews.llvm.org/D17339