[GlobalISel][Legalizer] Widening the second src op of shifts bug fix

[rtereshin]

Summary

The second source operand of G_SHL, G_ASHR, and G_LSHR must preserve its
value as a (small) unsigned integer, therefore its incorrect to widen it
in any way but by zero extending it.

G_SHL was using G_ANYEXT and G_ASHR - G_SEXT (which is correct for their
destination and first source operands, but not the "number of bits to
shift" operand).

Generally, shifts aren't as similar to regular binary operations as it
might seem, for instance, they aren't commutative nor associative and
the second source operand usually requires a special treatment.

[bogner]

Looks good, but please use update_llc_test_checks for the existing tests and move the test with the explanation to its own file

[rtereshin]

Updated x86 tests with the script and moved my own into a separate file.

[rtereshin]

Oops, squashed this with https://reviews.llvm.org/D46414 accidentally, changing it back.

[bogner]

lgtm, thanks!

[rtereshin]

@bogner Thank you!

llvm/trunk/lib/CodeGen/GlobalISel/LegalizerHelper.cpp

default:
     return UnableToLegalize;
 
   case TargetOpcode::G_ADD:
   case TargetOpcode::G_AND:
   case TargetOpcode::G_MUL:
   case TargetOpcode::G_OR:
   case TargetOpcode::G_XOR:
   case TargetOpcode::G_SUB:
-  case TargetOpcode::G_SHL:
     // Perform operation at larger width (any extension is fine here, high bits
     // don't affect the result) and then truncate the result back to the
     // original type.
     widenScalarSrc(MI, WideTy, 1, TargetOpcode::G_ANYEXT);
     widenScalarSrc(MI, WideTy, 2, TargetOpcode::G_ANYEXT);
     widenScalarDst(MI, WideTy);
     MIRBuilder.recordInsertion(&MI);
     return Legalized;
 
+  case TargetOpcode::G_SHL:
+    widenScalarSrc(MI, WideTy, 1, TargetOpcode::G_ANYEXT);
+    // The "number of bits to shift" operand must preserve its value as an
+    // unsigned integer:
+    widenScalarSrc(MI, WideTy, 2, TargetOpcode::G_ZEXT);
+    widenScalarDst(MI, WideTy);
+    MIRBuilder.recordInsertion(&MI);
+    return Legalized;
+
   case TargetOpcode::G_SDIV:
   case TargetOpcode::G_SREM:
-  case TargetOpcode::G_ASHR:
     widenScalarSrc(MI, WideTy, 1, TargetOpcode::G_SEXT);
     widenScalarSrc(MI, WideTy, 2, TargetOpcode::G_SEXT);
     widenScalarDst(MI, WideTy);
     MIRBuilder.recordInsertion(&MI);
     return Legalized;
 
+  case TargetOpcode::G_ASHR:
+    widenScalarSrc(MI, WideTy, 1, TargetOpcode::G_SEXT);
+    // The "number of bits to shift" operand must preserve its value as an
+    // unsigned integer:
+    widenScalarSrc(MI, WideTy, 2, TargetOpcode::G_ZEXT);
+    widenScalarDst(MI, WideTy);
+    MIRBuilder.recordInsertion(&MI);
+    return Legalized;
+
   case TargetOpcode::G_UDIV:
   case TargetOpcode::G_UREM:
   case TargetOpcode::G_LSHR:
     widenScalarSrc(MI, WideTy, 1, TargetOpcode::G_ZEXT);
     widenScalarSrc(MI, WideTy, 2, TargetOpcode::G_ZEXT);
     widenScalarDst(MI, WideTy);
     MIRBuilder.recordInsertion(&MI);
     return Legalized;

llvm/trunk/test/CodeGen/AArch64/GlobalISel/legalize-shift.mir

bb.0.entry:
     liveins: $x0, $x1, $x2, $x3
     ; CHECK-LABEL: name: test_shift
     ; CHECK: [[COPY:%[0-9]+]]:_(s64) = COPY $x0
     ; CHECK: [[COPY1:%[0-9]+]]:_(s64) = COPY $x1
     ; CHECK: [[C:%[0-9]+]]:_(s32) = G_CONSTANT i32 24
     ; CHECK: [[TRUNC:%[0-9]+]]:_(s32) = G_TRUNC [[COPY]](s64)
     ; CHECK: [[SHL:%[0-9]+]]:_(s32) = G_SHL [[TRUNC]], [[C]]
     ; CHECK: [[ASHR:%[0-9]+]]:_(s32) = G_ASHR [[SHL]], [[C]]
-    ; CHECK: [[C1:%[0-9]+]]:_(s32) = G_CONSTANT i32 24
+    ; CHECK: [[C1:%[0-9]+]]:_(s32) = G_CONSTANT i32 255
     ; CHECK: [[TRUNC1:%[0-9]+]]:_(s32) = G_TRUNC [[COPY1]](s64)
-    ; CHECK: [[SHL1:%[0-9]+]]:_(s32) = G_SHL [[TRUNC1]], [[C1]]
-    ; CHECK: [[ASHR1:%[0-9]+]]:_(s32) = G_ASHR [[SHL1]], [[C1]]
-    ; CHECK: [[ASHR2:%[0-9]+]]:_(s32) = G_ASHR [[ASHR]], [[ASHR1]]
-    ; CHECK: [[COPY2:%[0-9]+]]:_(s32) = COPY [[ASHR2]](s32)
+    ; CHECK: [[AND:%[0-9]+]]:_(s32) = G_AND [[TRUNC1]], [[C1]]
+    ; CHECK: [[ASHR1:%[0-9]+]]:_(s32) = G_ASHR [[ASHR]], [[AND]]
+    ; CHECK: [[COPY2:%[0-9]+]]:_(s32) = COPY [[ASHR1]](s32)
     ; CHECK: $w0 = COPY [[COPY2]](s32)
     ; CHECK: [[C2:%[0-9]+]]:_(s32) = G_CONSTANT i32 255
     ; CHECK: [[TRUNC2:%[0-9]+]]:_(s32) = G_TRUNC [[COPY]](s64)
-    ; CHECK: [[AND:%[0-9]+]]:_(s32) = G_AND [[TRUNC2]], [[C2]]
+    ; CHECK: [[AND1:%[0-9]+]]:_(s32) = G_AND [[TRUNC2]], [[C2]]
     ; CHECK: [[C3:%[0-9]+]]:_(s32) = G_CONSTANT i32 255
     ; CHECK: [[TRUNC3:%[0-9]+]]:_(s32) = G_TRUNC [[COPY1]](s64)
-    ; CHECK: [[AND1:%[0-9]+]]:_(s32) = G_AND [[TRUNC3]], [[C3]]
-    ; CHECK: [[LSHR:%[0-9]+]]:_(s32) = G_LSHR [[AND]], [[AND1]]
+    ; CHECK: [[AND2:%[0-9]+]]:_(s32) = G_AND [[TRUNC3]], [[C3]]
+    ; CHECK: [[LSHR:%[0-9]+]]:_(s32) = G_LSHR [[AND1]], [[AND2]]
     ; CHECK: [[COPY3:%[0-9]+]]:_(s32) = COPY [[LSHR]](s32)
     ; CHECK: $w0 = COPY [[COPY3]](s32)
     ; CHECK: [[TRUNC4:%[0-9]+]]:_(s32) = G_TRUNC [[COPY]](s64)
+    ; CHECK: [[C4:%[0-9]+]]:_(s32) = G_CONSTANT i32 255
     ; CHECK: [[TRUNC5:%[0-9]+]]:_(s32) = G_TRUNC [[COPY1]](s64)
-    ; CHECK: [[SHL2:%[0-9]+]]:_(s32) = G_SHL [[COPY1]]0, [[COPY1]]1
-    ; CHECK: [[COPY4:%[0-9]+]]:_(s32) = COPY [[COPY1]]2(s32)
+    ; CHECK: [[AND3:%[0-9]+]]:_(s32) = G_AND [[TRUNC5]], [[C4]]
+    ; CHECK: [[SHL1:%[0-9]+]]:_(s32) = G_SHL [[TRUNC4]], [[AND3]]
+    ; CHECK: [[COPY4:%[0-9]+]]:_(s32) = COPY [[SHL1]](s32)
     ; CHECK: $w0 = COPY [[COPY4]](s32)
     %0(s64) = COPY $x0
     %1(s64) = COPY $x1
     %2(s8) = G_TRUNC %0
     %3(s8) = G_TRUNC %1
 
     %4(s8) = G_ASHR %2, %3
     %7:_(s32) = G_ANYEXT %4

llvm/trunk/test/CodeGen/X86/GlobalISel/ashr-scalar.ll

; X64-NEXT:    retq
   ret i8 %res
 }
 
 define i1 @test_ashr_i1(i32 %arg1, i32 %arg2) {
 ; X64-LABEL: test_ashr_i1:
 ; X64:       # %bb.0:
 ; X64-NEXT:    shlb $7, %dil
 ; X64-NEXT:    sarb $7, %dil
-; X64-NEXT:    shlb $7, %sil
-; X64-NEXT:    sarb $7, %sil
+; X64-NEXT:    andb $1, %sil
 ; X64-NEXT:    movl %esi, %ecx
 ; X64-NEXT:    sarb %cl, %dil
 ; X64-NEXT:    movl %edi, %eax
 ; X64-NEXT:    retq
   %a = trunc i32 %arg1 to i1
   %a2 = trunc i32 %arg2 to i1
   %res = ashr i1 %a, %a2
   ret i1 %res
 }
 
 define i1 @test_ashr_i1_imm1(i32 %arg1) {
 ; X64-LABEL: test_ashr_i1_imm1:
 ; X64:       # %bb.0:
 ; X64-NEXT:    movb $-1, %cl
 ; X64-NEXT:    shlb $7, %dil
 ; X64-NEXT:    sarb $7, %dil
-; X64-NEXT:    shlb $7, %cl
-; X64-NEXT:    sarb $7, %cl
+; X64-NEXT:    andb $1, %cl
 ; X64-NEXT:    sarb %cl, %dil
 ; X64-NEXT:    movl %edi, %eax
 ; X64-NEXT:    retq
   %a = trunc i32 %arg1 to i1
   %res = ashr i1 %a, 1
   ret i1 %res
 }

llvm/trunk/test/CodeGen/X86/GlobalISel/shl-scalar-widening.ll

+; RUN: llc -mtriple=x86_64-linux-gnu -global-isel -verify-machineinstrs < %s -o - | FileCheck %s --check-prefix=X64
+
+define i16 @test_shl_i4(i16 %v, i16 %a, i16 %b) {
+; Let's say the arguments are the following unsigned
+; integers in two’s complement representation:
+;
+; %v: 77 (0000 0000  0100 1101)
+; %a: 74 (0000 0000  0100 1010)
+; %b: 72 (0000 0000  0100 1000)
+  %v.t = trunc i16 %v to i4  ; %v.t: 13 (1101)
+  %a.t = trunc i16 %a to i4  ; %a.t: 10 (1010)
+  %b.t = trunc i16 %b to i4  ; %b.t:  8 (1000)
+  %n.t = add i4 %a.t, %b.t   ; %n.t:  2 (0010)
+  %r.t = shl i4 %v.t, %n.t   ; %r.t:  4 (0100)
+  %r = zext i4 %r.t to i16
+; %r:  4 (0000 0000 0000 0100)
+  ret i16 %r
+
+; X64-LABEL: test_shl_i4
+;
+; %di:  77 (0000 0000  0100 1101)
+; %si:  74 (0000 0000  0100 1010)
+; %dx:  72 (0000 0000  0100 1000)
+;
+; X64:       # %bb.0:
+;
+; X64-NEXT:    addb %sil, %dl
+; %dx: 146 (0000 0000  1001 0010)
+;
+; X64-NEXT:    andb $15, %dl
+; %dx:   2 (0000 0000  0000 0010)
+;
+; X64-NEXT:    movl %edx, %ecx
+; %cx:   2 (0000 0000  0000 0010)
+;
+; X64-NEXT:    shlb %cl, %dil
+; %di:  52 (0000 0000  0011 0100)
+;
+; X64-NEXT:    andw $15, %di
+; %di:   4 (0000 0000  0000 0100)
+;
+; X64-NEXT:    movl %edi, %eax
+; %ax:   4 (0000 0000  0000 0100)
+;
+; X64-NEXT:    retq
+;
+; Let's pretend that legalizing G_SHL by widening its second
+; source operand is done via G_ANYEXT rather than G_ZEXT and
+; see what happens:
+;
+;              addb %sil, %dl
+; %dx: 146 (0000 0000  1001 0010)
+;
+;              movl %edx, %ecx
+; %cx: 146 (0000 0000  1001 0010)
+;
+;              shlb %cl, %dil
+; %di:   0 (0000 0000  0000 0000)
+;
+;              andw $15, %di
+; %di:   0 (0000 0000  0000 0000)
+;
+;              movl %edi, %eax
+; %ax:   0 (0000 0000  0000 0000)
+;
+;              retq
+}

llvm/trunk/test/CodeGen/X86/GlobalISel/shl-scalar.ll

; X64-NEXT:    retq
   %a = trunc i32 %arg1 to i8
   %res = shl i8 %a, 1
   ret i8 %res
 }
 
 define i1 @test_shl_i1(i32 %arg1, i32 %arg2) {
 ; X64-LABEL: test_shl_i1:
 ; X64:       # %bb.0:
+; X64-NEXT:    andb $1, %sil
 ; X64-NEXT:    movl %esi, %ecx
 ; X64-NEXT:    shlb %cl, %dil
 ; X64-NEXT:    movl %edi, %eax
 ; X64-NEXT:    retq
   %a = trunc i32 %arg1 to i1
   %a2 = trunc i32 %arg2 to i1
   %res = shl i1 %a, %a2
   ret i1 %res
 }
 
 define i1 @test_shl_i1_imm1(i32 %arg1) {
 ; X64-LABEL: test_shl_i1_imm1:
 ; X64:       # %bb.0:
 ; X64-NEXT:    movb $-1, %cl
+; X64-NEXT:    andb $1, %cl
 ; X64-NEXT:    shlb %cl, %dil
 ; X64-NEXT:    movl %edi, %eax
 ; X64-NEXT:    retq
   %a = trunc i32 %arg1 to i1
   %res = shl i1 %a, 1
   ret i1 %res
 }