[PATCH] D21372: [ARM] Lower (select_cc k k (select_cc ~k ~k x)) into (SSAT l_k x)

[Renato Golin via llvm-commits]

rengolin accepted this revision.
rengolin added a reviewer: rengolin.
rengolin added a comment.
This revision is now accepted and ready to land.

In http://reviews.llvm.org/D21372#464862, @pbarrio wrote:

> Second round of changes addressed. Thanks!


Thanks! Looks good to me now, + using std::max.

> Also a side-question. Would it be better to use std::max(V1, V2) instead of

>  V1 > V2 ? V1 : V2? If so, why?


Yes, because I first assumed it was it, so didn't bothered. It was only when I looked again that it didn't make sense because the condition was reversed but the values were unsigned. While it could potentially work on all cases, it was an obtuse construct, and prone to confusion.

If you had used std::min from the beginning, I'd have caught it early on, while I could have missed it with the ternary.

>From a code generation point of view, std::max/min generates the exact same code as the ternary, so there's no reason to not use it.

cheers,
--renato


http://reviews.llvm.org/D21372