[all-commits] [llvm/llvm-project] de2378: [X86] Fix a KNL miscompile caused by combineSetCC ...

[topperc via All-commits]

  Branch: refs/heads/master
  Home:   https://github.com/llvm/llvm-project
  Commit: de2378b4f3c4c88eae412a30a8bfaec82f54d1b7
      https://github.com/llvm/llvm-project/commit/de2378b4f3c4c88eae412a30a8bfaec82f54d1b7
  Author: Craig Topper <craig.topper at intel.com>
  Date:   2019-12-20 (Fri, 20 Dec 2019)

  Changed paths:
    M llvm/lib/Target/X86/X86ISelLowering.cpp
    M llvm/test/CodeGen/X86/avx512-vec-cmp.ll

  Log Message:
  -----------
  [X86] Fix a KNL miscompile caused by combineSetCC swapping LHS/RHS variables before a later use.

The setcc operands are copied into LHS and RHS variables at the top of the function. We also capture the condition code.

A later piece of code swaps the operands and changing the CC variable as part of a canonicalization to make some other checks simpler. But we might not make the transform we canonicalized for. So we continue on through the function where we can use the swapped LHS/RHS variables and access the original condition code operand instead of the modified CC variable. This leads to a setcc being created with the original condition code, but with swapped operands.

To mitigate this, this patch does a couple things. The LHS/RHS/CC variables are made const to keep them from being modified like this again. The transform that needs the swap now uses temporary copies of the variables. And the transform that used the original condition code operand has been altered to use the CC variable we cached originally. Either of these changes are enough to fix the issue, but doing both to make this code very safe.

I also considered rewriting the swap code in some way to check both permutations without explicitly swapping or needing temporary variables, but held off on that.

Differential Revision: https://reviews.llvm.org/D71736