[PATCH] D37504: [x86] Fix PR34377 by disabling cmov conversion when we relied on it performing a zext of a register.

[Chandler Carruth via Phabricator via llvm-commits]

chandlerc added inline comments.


================
Comment at: llvm/trunk/lib/Target/X86/X86CmovConversion.cpp:297
+                [&](MachineInstr &UseI) {
+                  return UseI.getOpcode() == X86::SUBREG_TO_REG;
+                }))
----------------
aaboud wrote:
> Thanks Chandler for the quick fix.
> However, we can reduce the restriction to only the case where:
> 1. We are compiling for 64bit
> 2. The CMOV destination is 32bit.
> 
> All other cases has no issue with the CMOV behavior.
> 
> Do you agree?
Hmm. Probably, because that's where zext is present.

However, aren't those the same conditions in which SUBREG_TO_REG will be introduced? If this is an assertion of zext-ing behavior, it can only show up due to there being zext-ing behavior of cmov itself, and that seems to be the same set of restrictions you outline.

Not sure how much more time we should spend on enhanceing the dodge of a miscompile vs. the perhaps more interesting work to handle this case elegantly and effectively.


Repository:
  rL LLVM

https://reviews.llvm.org/D37504