Fix Logical Right Shift Simplify Bug:

[Michael Kruse via llvm-commits]

2016-08-05 11:21 GMT+02:00 林作健 via llvm-commits <llvm-commits at lists.llvm.org>:
> Problem code:
> bool fits_shifter(uint32_t imm32,
>                          uint32_t* rotate_imm,
>                          uint32_t* immed_8,
>                          Instr* instr) {
>   // imm32 must be unsigned.
>   for (int rot = 0; rot < 16; rot++) {
>     uint32_t imm8 = (imm32 << 2*rot) | (imm32 >> (32 - 2*rot));
>     if ((imm8 <= 0xff)) {
>       *rotate_imm = rot;
>       *immed_8 = imm8;
>       return true;
>     }
>   }
>   return false;
> }
>
> After loop unroll pass, which has never been enabled before 3.8, the llvm
> IR:
> for.body:                                         ; preds = %tailrecurse
>   br i1 false, label %cleanup3, label %for.inc
>
> for.inc:                                          ; preds = %for.body
>   %shl.1 = shl i32 %imm32.tr, 2
>   %shr.1 = lshr i32 %imm32.tr, 30
>   %or.1 = or i32 %shr.1, %shl.1
>   %cmp2.1 = icmp ult i32 %or.1, 256
>   br i1 %cmp2.1, label %cleanup3, label %for.inc.1
>
> As you see, the for.body block becomes a trivial block, which
> is caused by instruction simplify, like the following:
>
> for.body:                                         ; preds = %tailrecurse
>   %mul = shl nsw i32 0, 1
>   %shl = shl i32 %imm32.tr, %mul
>   %sub = sub nuw nsw i32 32, %mul
>   %shr = lshr i32 %imm32.tr, %sub
>   %or = or i32 %shr, %shl
>   %cmp2 = icmp ult i32 %or, 256
>   br i1 %cmp2, label %cleanup3, label %for.inc
>
>
> When simplifying  %shr = lshr i32 %imm32.tr, %sub with %sub is a constant
> 32,
> the value undefined will be generated. Then %or = or i32 %shr, %shl will
> generate a
> constant -1, and then cause the whole block trivially branch to the negative
> branch.
>
> My patch in the attachment tries to fix this by generating constant 0 when
> the right shift exceeds
> the types bit limits. And That's what is expecting in the real life
> scenario.

Bit shift by the type's bitsize or more is undefined behavior in C/C++
(C11, 6.5.7p3; C++11, [expr.shift] 2.). If this happens the compiler
can do anything it likes to ("make fly demons out of your nose"). LLVM
chooses to use Undef. You should fix your original code, eg. introduce
a separate case for rot==16 and rot==0.

With this 'fix' the output program might behave differently on
different CPUs, depending on how they implement shift operations.
Common implementations are to shift on the shift amount modulo type
size (ie. a nop when the shift amount is equal to the size) or to set
the result to all-zeros or all-ones (ashr on negative values).

Michael