[llvm-bugs] [Bug 43689] New: [instcombine] Miscompile in visitShuffleVectorInst (transform introduces unsafe integer div/rem)

[via llvm-bugs]

https://bugs.llvm.org/show_bug.cgi?id=43689

            Bug ID: 43689
           Summary: [instcombine] Miscompile in visitShuffleVectorInst
                    (transform introduces unsafe integer div/rem)
           Product: libraries
           Version: trunk
          Hardware: PC
                OS: Windows NT
            Status: NEW
          Severity: enhancement
          Priority: P
         Component: Scalar Optimizations
          Assignee: unassignedbugs at nondot.org
          Reporter: bjorn.a.pettersson at ericsson.com
                CC: llvm-bugs at lists.llvm.org

Our downstream fuzzy test framework found a miscompile due to what seems to be
a bug in InstCombine.

Here is a reduced test case:

;-------------------------------------------------------------------------------------------------
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
; RUN: opt < %s -instcombine -S -o - | FileCheck %s

; This test case was added as a reproducer for a miscompile, where instcombine
; introduced an
;   srem <2 x i16> %1, <i16 undef, i16 2>
; instruction, which makes the whole srem undefined (even if we only end up
; extracting the second element in the vector).
define i16 @test1(i16 %a, i1 %cmp) {
; CHECK-LABEL: @test1(
; CHECK-NEXT:    ret i16 1
;
  %splatinsert = insertelement <2 x i16> undef, i16 %a, i32 0
  %splat = shufflevector <2 x i16> %splatinsert, <2 x i16> undef, <2 x i32>
zeroinitializer
  %t1 = select i1 %cmp, <2 x i16> <i16 1, i16 1>, <2 x i16> %splat
  %t2 = srem <2 x i16> %t1, <i16 2, i16 2>
  %t3 = extractelement <2 x i16> %t2, i32 1
  ret i16 %t3
}

; This is basically a reduced version of test1 (based on what the code would
; look like after a few iterations of instcombine, just before we try to
; transform the shufflevector by doing "evaluateInDifferentElementOrder".
define <2 x i16> @test2(i16 %a, i1 %cmp) {
; CHECK-LABEL: @test2(
; CHECK-NEXT:    ret <2 x i16> <i16 77, i16 99>
;
  %splatinsert = insertelement <2 x i16> undef, i16 %a, i32 0
  %t1 = srem <2 x i16> %splatinsert, <i16 2, i16 1>
  %splat.op = shufflevector <2 x i16> %t1, <2 x i16> undef, <2 x i32> <i32
undef, i32 0>
  %t2 = select i1 %cmp, <2 x i16> <i16 77, i16 99>, <2 x i16> %splat.op
  ret <2 x i16> %t2
}
;-------------------------------------------------------------------------------------------------


Problem is that instcombine transforms IR like this

  %splatinsert = insertelement <2 x i16> undef, i16 %a, i32 0
  %t1 = srem <2 x i16> %splatinsert, <i16 2, i16 1>
  %splat.op = shufflevector <2 x i16> %t1, <2 x i16> undef, <2 x i32> <i32
undef, i32 0>

into

   %1 = insertelement <2 x i16> undef, i16 %a, i32 1
   %2 = srem <2 x i16> %1, <i16 undef, i16 2>

which introduces an undef denominator in the srem. This makes the whole srem
undefined and we get a miscompile.

The transform above happens in InstCombiner::visitShuffleVectorInst when doing 

  if (isa<UndefValue>(RHS) && canEvaluateShuffled(LHS, Mask)) {
    Value *V = evaluateInDifferentElementOrder(LHS, Mask);
    return replaceInstUsesWith(SVI, V);
  }

I think the bug can be solved by adding some more restrictions in
canEvaluateShuffled for integer div/rem instructions.
I have a suggested patch that I'll upload in Phabricator.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-bugs/attachments/20191016/6f603688/attachment.html>