[llvm-bugs] [Bug 48353] New: [InstCombine] Incorrect folding of LShr into select instruction

[via llvm-bugs]

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

            Bug ID: 48353
           Summary: [InstCombine] Incorrect folding of LShr into select
                    instruction
           Product: libraries
           Version: trunk
          Hardware: PC
                OS: Linux
            Status: NEW
          Severity: enhancement
          Priority: P
         Component: Scalar Optimizations
          Assignee: unassignedbugs at nondot.org
          Reporter: congzhecao at gmail.com
                CC: llvm-bugs at lists.llvm.org

The IR that exposes this bug is as follows. 

; ModuleID = 'test.c'

@m = common dso_local local_unnamed_addr global i32 0, align 4

define dso_local i32 @test() local_unnamed_addr #1 {
entry:
  call fastcc void @foo(i32 51)
  ret i32 0
}

define internal fastcc void @foo(i32 %aj) {
entry:
  %cmp.i = icmp sgt i32 %aj, 1
  %shr.i = select i1 %cmp.i, i32 0, i32 %aj
  %cond.i1 = lshr i32 3, %shr.i
  %conv.i = trunc i32 %cond.i1 to i8
  %0 = and i8 %conv.i, 1
  %cmp4 = icmp ugt i8 %0, 0
  %conv5 = zext i1 %cmp4 to i32
  store i32 %conv5, i32* @m
  ret void
}

Note that at compile time we know the argument of foo() is %aj = 51 thus
"%shr.i = select i1 %cmp.i, i32 0, i32 %aj" will result in 0 and hence LShr
instruction that follows is correct, not resulting in a poison value.

When running "opt -instcombine test.ll -S", the following IR is generated.
Specifically, "%aj.op = lshr i32 3, %aj" is generated which result in a poison
value since %aj is actually greater than 32.

@m = common dso_local local_unnamed_addr global i32 0, align 4

define dso_local i32 @test() local_unnamed_addr #1 {
entry:
  call fastcc void @foo(i32 51)
  ret i32 0
}

define internal fastcc void @o(i32 %aj) {
entry:
  %cmp.i = icmp sgt i32 %aj, 1
  %aj.op = lshr i32 3, %aj
  %.op2 = and i32 %aj.op, 1
  %cmp.i3 = zext i1 %cmp.i to i32
  %conv55 = or i32 %.op2, %cmp.i3
  store i32 %conv55, i32* @m, align 4
  ret void
}

************************

The cause is that during InstCombine, we fold 

%shr.i = select i1 %cmp.i, i32 0, i32 %aj
%cond.i1 = lshr i32 3, %shr.i

into 

%aj.op = lshr i32 3, %aj       
%cond.i1 = select i1 %cmp.i, i32 3, i32 %aj.op,

thus generating the incorrect LShr instruction "%aj.op = lshr i32 3, %aj".

****************************

Our proposed fix:

In function InstCombiner::FoldOpIntoSelect() from
Transforms/InstCombine/InstructionCombining.cpp, add the following check such
that we only fold LShr into select when both the true value and the false value
are known to be constants. 

  // If op is a LShr instruction, we can only fold into select when both TV
  // and FV are known to be constants, otherwise they may be evaluated to be
  // greater than the width of the first operand of LShr, resulting in for
  // example:
  //     LShr i32 X Y where Y > 32,
  // which is not suppose to be generated.
  if (Op.getOpcode() == Instruction::LShr &&
      !(isa<Constant>(TV) && isa<Constant>(FV))) {
    return nullptr;
  }

-- 
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/20201201/64582425/attachment.html>