[PATCH] D36711: [X86] Combining CMOVs with [ANY, SIGN, ZERO]_EXTEND for cases where CMOV has constant arguments

[Sanjay Patel via Phabricator via llvm-commits]

spatel added a comment.

In https://reviews.llvm.org/D36711#842468, @aivchenk wrote:

> Thank you for your comments, I updated the patch to address them:
>
> 1. Done
> 2. MCU target does not have CMOV instruction so it is always converted into test+branch. I firstly cared about it - that's the reason why it is here


I don't have a good understanding of this path then. Why are we generating x86-specific cmov nodes for a target that doesn't support cmov? I would think these should remain as select nodes to avoid creating a flags result that won't exist, but I must be missing something?

> About converting into math ops.. I think that in most cases my patch would be obsolete then. The only reason I see why it would be not obsolete is: assume that CMOV with constants is part of a CMOV-group (as defined in X86CmovConversion.cpp: consecutive/having the same CC) and the decision taken by x86-cmov-converter is to transform this CMOV-group into branch. Since branch is generated anyways now, basically this "constant" CMOV is transformed into two addition MOVs, which should be more effective than converting it to three ops math logic

That's a good point - if the compare has multiple uses, we probably shouldn't convert to math. This may already be wrong in the existing transforms. It's also possible that a cmov sequence is faster (the dependency chain is shorter), but I think a CPU would have to be able to execute cmov at the same speed/throughput as the simple ops for that to be true.


https://reviews.llvm.org/D36711