svn commit: r337615 - head/contrib/llvm/lib/Target/X86

[Dimitry Andric]

Author: dim
Date: Sat Aug 11 10:42:12 2018
New Revision: 337615
URL: https://svnweb.freebsd.org/changeset/base/337615

Log:
  Pull in r338481 from upstream llvm trunk (by Chandler Carruth):
  
    [x86] Fix a really subtle miscompile due to a somewhat glaring bug in
    EFLAGS copy lowering.
  
    If you have a branch of LLVM, you may want to cherrypick this. It is
    extremely unlikely to hit this case empirically, but it will likely
    manifest as an "impossible" branch being taken somewhere, and will be
    ... very hard to debug.
  
    Hitting this requires complex conditions living across complex
    control flow combined with some interesting memory (non-stack)
    initialized with the results of a comparison. Also, because you have
    to arrange for an EFLAGS copy to be in *just* the right place, almost
    anything you do to the code will hide the bug. I was unable to reduce
    anything remotely resembling a "good" test case from the place where
    I hit it, and so instead I have constructed synthetic MIR testing
    that directly exercises the bug in question (as well as the good
    behavior for completeness).
  
    The issue is that we would mistakenly assume any SETcc with a valid
    condition and an initial operand that was a register and a virtual
    register at that to be a register *defining* SETcc...
  
    It isn't though....
  
    This would in turn cause us to test some other bizarre register,
    typically the base pointer of some memory. Now, testing this register
    and using that to branch on doesn't make any sense. It even fails the
    machine verifier (if you are running it) due to the wrong register
    class. But it will make it through LLVM, assemble, and it *looks*
    fine... But wow do you get a very unsual and surprising branch taken
    in your actual code.
  
    The fix is to actually check what kind of SETcc instruction we're
    dealing with. Because there are a bunch of them, I just test the
    may-store bit in the instruction. I've also added an assert for
    sanity that ensure we are, in fact, *defining* the register operand.
    =D
  
  Noticed by:	kib
  MFC after:	1 week

Modified:
  head/contrib/llvm/lib/Target/X86/X86FlagsCopyLowering.cpp

Modified: head/contrib/llvm/lib/Target/X86/X86FlagsCopyLowering.cpp
==============================================================================
--- head/contrib/llvm/lib/Target/X86/X86FlagsCopyLowering.cpp	Sat Aug 11 10:21:21 2018	(r337614)
+++ head/contrib/llvm/lib/Target/X86/X86FlagsCopyLowering.cpp	Sat Aug 11 10:42:12 2018	(r337615)
@@ -608,9 +608,12 @@ X86FlagsCopyLoweringPass::collectCondsInRegs(MachineBa
   for (MachineInstr &MI : llvm::reverse(
            llvm::make_range(MBB.instr_begin(), CopyDefI.getIterator()))) {
     X86::CondCode Cond = X86::getCondFromSETOpc(MI.getOpcode());
-    if (Cond != X86::COND_INVALID && MI.getOperand(0).isReg() &&
-        TRI->isVirtualRegister(MI.getOperand(0).getReg()))
+    if (Cond != X86::COND_INVALID && !MI.mayStore() && MI.getOperand(0).isReg() &&
+        TRI->isVirtualRegister(MI.getOperand(0).getReg())) {
+      assert(MI.getOperand(0).isDef() &&
+             "A non-storing SETcc should always define a register!");
       CondRegs[Cond] = MI.getOperand(0).getReg();
+    }
 
     // Stop scanning when we see the first definition of the EFLAGS as prior to
     // this we would potentially capture the wrong flag state.