[PATCH] D134892: [X86] Avoid miscompile in combineOr (X86ISelLowering.cpp)

[Bjorn Pettersson via Phabricator via llvm-commits]

bjope created this revision.
bjope added reviewers: craig.topper, RKSimon, deadalnix.
Herald added subscribers: StephenFan, pengfei, hiraditya.
Herald added a project: All.
bjope requested review of this revision.
Herald added a project: LLVM.

In combineOr (X86ISelLowering.cpp) there is a DAG combine that rewrite
a "(0 - SetCC) | C" pattern into something simpler given that a LEA
can be used. Another requirement is that C has some specific value,
for example 1 or 7. When checking those requirements the code used a
32-bit unsigned variable to store the value of C. So for a 64-bit OR
this could miscompile in case any of the 32 most significant bits in
C were non zero.

This patch adds fixes the bug by using a large enough type for the
C value.

The faulty code seem to have been introduced by commit 9bceb8981d32fe <https://reviews.llvm.org/rG9bceb8981d32fe9465257c31413395f445ab05d8>
(D131358 <https://reviews.llvm.org/D131358>).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D134892

Files:
  llvm/lib/Target/X86/X86ISelLowering.cpp
  llvm/test/CodeGen/X86/or-lea.ll


Index: llvm/test/CodeGen/X86/or-lea.ll
===================================================================
--- llvm/test/CodeGen/X86/or-lea.ll
+++ llvm/test/CodeGen/X86/or-lea.ll
@@ -811,10 +811,12 @@
 ;
 ; X64-LABEL: or_large_constant:
 ; X64:       # %bb.0: # %entry
-; X64-NEXT:    xorl %eax, %eax
+; X64-NEXT:    xorl %ecx, %ecx
 ; X64-NEXT:    cmpq $2, %rdi
-; X64-NEXT:    setl %al
-; X64-NEXT:    leaq -1(%rax,%rax), %rax
+; X64-NEXT:    setge %cl
+; X64-NEXT:    negq %rcx
+; X64-NEXT:    movabsq $549755813889, %rax # imm = 0x8000000001
+; X64-NEXT:    orq %rcx, %rax
 ; X64-NEXT:    retq
 entry:
   %cmp = icmp sgt i64 %x, 1
Index: llvm/lib/Target/X86/X86ISelLowering.cpp
===================================================================
--- llvm/lib/Target/X86/X86ISelLowering.cpp
+++ llvm/lib/Target/X86/X86ISelLowering.cpp
@@ -48592,7 +48592,7 @@
 
     if (Cond.getOpcode() == X86ISD::SETCC && Cond.hasOneUse()) {
       if (auto *CN = dyn_cast<ConstantSDNode>(N1)) {
-        unsigned Val = CN->getZExtValue();
+        uint64_t Val = CN->getZExtValue();
         if (Val == 1 || Val == 2 || Val == 3 || Val == 4 || Val == 7 || Val == 8) {
           X86::CondCode CCode = (X86::CondCode)Cond.getConstantOperandVal(0);
           CCode = X86::GetOppositeBranchCondition(CCode);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D134892.463936.patch
Type: text/x-patch
Size: 1295 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20220929/2cafe4cb/attachment.bin>