Details

This canonicalization was suggested in D33172 as a way to make InstCombine behavior more uniform. We have this transform for icmp+br, so unless there's some reason that icmp+select should be treated differently, I think we should do the same thing here.

The benefit comes from increasing the chances of creating identical instructions. This is shown in the tests in logical-select.ll (PR32791). InstCombine doesn't fold those directly, but EarlyCSE can simplify the identical cmps, and then InstCombine can fold the selects together.

...but I think that transform is just as likely to be triggered by this canonicalization as it is to be missed, so we're just pointing out a commutation deficiency in the pattern matching:https://reviews.llvm.org/rL228409

Are you suggesting a change to GVN as an independent improvement or as a substitute for this transform in InstCombine? As I mentioned, I think this patch makes InstCombine behavior more predictable and consistent (because we already have this transform for cmp+br).