Fix UShr/Shr(Shl(x, N), N) simplification

Do not introduce new implicit type conversions during simplification
otherwise we could have unexpected pattern

<<ImplicitConv>> TypeConversion
<<ExplicitConv>> TypeConversonn [<<ImplicitConv>>]

that leads to crash.

Test: testrunner.py --optimizing
Bug: 323462814
Change-Id: I7f2cb7e778712e86f3e41b86945a52eac7036050

2 files changed, 233 insertions, 5 deletions

diff --git a/compiler/optimizing/instruction_simplifier.cc b/compiler/optimizing/instruction_simplifier.cc
index 5db8235e29..44d624883d 100644
--- a/compiler/optimizing/instruction_simplifier.cc
+++ b/compiler/optimizing/instruction_simplifier.cc
@@ -450,15 +450,22 @@ static bool TryReplaceShiftsByConstantWithTypeConversion(HBinaryOperation *instr
   bool is_signed = instruction->IsShr();
   DataType::Type conv_type =
       is_signed ? source_integral_type : DataType::ToUnsigned(source_integral_type);
+
+  DCHECK(DataType::IsTypeConversionImplicit(conv_type, instruction->GetResultType()));
+
   HInstruction* shl_value = shl->GetLeft();
   HBasicBlock *block = instruction->GetBlock();
 
-  HTypeConversion* new_conversion =
-      new (block->GetGraph()->GetAllocator()) HTypeConversion(conv_type, shl_value);
-
-  DCHECK(DataType::IsTypeConversionImplicit(conv_type, instruction->GetResultType()));
+  // We shouldn't introduce new implicit type conversions during simplification.
+  if (DataType::IsTypeConversionImplicit(shl_value->GetType(), conv_type)) {
+    instruction->ReplaceWith(shl_value);
+    instruction->GetBlock()->RemoveInstruction(instruction);
+  } else {
+    HTypeConversion* new_conversion =
+        new (block->GetGraph()->GetAllocator()) HTypeConversion(conv_type, shl_value);
+    block->ReplaceAndRemoveInstructionWith(instruction, new_conversion);
+  }
 
-  block->ReplaceAndRemoveInstructionWith(instruction, new_conversion);
   shl->GetBlock()->RemoveInstruction(shl);
 
   return true;
diff --git a/test/458-checker-instruct-simplification/src/Main.java b/test/458-checker-instruct-simplification/src/Main.java
index 76239be741..784acf7949 100644
--- a/test/458-checker-instruct-simplification/src/Main.java
+++ b/test/458-checker-instruct-simplification/src/Main.java
@@ -3692,6 +3692,206 @@ public class Main {
     return arg << 25 >> 26;
   }
 
+  // Check that we don't introduce new implicit type conversions so the following pattern
+  // does not occur in the graph:
+  //
+  // <<ImplicitConv>> TypeConversion
+  // <<ExplicitConv>> TypeConversonn [<<ImplicitConv>>]
+  //
+  // That will lead to a crash because InstructionSimplifier removes implicit type conversions
+  // and during visiting TypeConversion instruction expects that its inputs have been already
+  // simplified.
+  //
+  // The structure of the following tests is
+  //
+  //   (T) ((x << N) >> N) or (T) ((x << N) >>> N)
+  //
+  // where
+  //   * K is a type of x
+  //   * Shifts correspond to implicit type conversion K -> M
+  //   * M -> T conversion is explicit
+  //
+  // T itself doesn't matter, the only important thing is that M -> T is explicit.
+  //
+  // We check cases when shifts correspond to the following implicit type conversions:
+  //   byte -> byte
+  //   byte -> short
+  //   unsigned byte -> unsigned byte
+  //   unsigned byte -> short
+  //   unsigned byte -> char
+  //   short -> short
+  //   char -> char
+  //
+  // To produce unsigned byte bitwise AND with 0xFF is used.
+
+  /// CHECK-START: int Main.$noinline$testByteToByteToChar(byte) instruction_simplifier (before)
+  /// CHECK:     <<Param:b\d+>>   ParameterValue
+  /// CHECK:     <<Const24:i\d+>> IntConstant 24
+  /// CHECK:     <<Shl:i\d+>>     Shl [<<Param>>,<<Const24>>]
+  /// CHECK:     <<Shr:i\d+>>     Shr [<<Shl>>,<<Const24>>]
+  /// CHECK:     <<Conv:c\d+>>    TypeConversion [<<Shr>>]
+  /// CHECK:     <<Return:v\d+>>  Return [<<Conv>>]
+
+  /// CHECK-START: int Main.$noinline$testByteToByteToChar(byte) instruction_simplifier (after)
+  /// CHECK:     <<Param:b\d+>>   ParameterValue
+  /// CHECK:     <<Conv:c\d+>>    TypeConversion [<<Param>>]
+  /// CHECK:     <<Return:v\d+>>  Return [<<Conv>>]
+
+  /// CHECK-START: int Main.$noinline$testByteToByteToChar(byte) instruction_simplifier (after)
+  /// CHECK-NOT:                  Shl
+
+  /// CHECK-START: int Main.$noinline$testByteToByteToChar(byte) instruction_simplifier (after)
+  /// CHECK-NOT:                  Shr
+  private static int $noinline$testByteToByteToChar(byte arg) {
+    return (char) ((arg << 24) >> 24);
+  }
+
+  /// CHECK-START: int Main.$noinline$testByteToShortToByte(byte) instruction_simplifier (before)
+  /// CHECK:     <<Param:b\d+>>   ParameterValue
+  /// CHECK:     <<Const16:i\d+>> IntConstant 16
+  /// CHECK:     <<Shl:i\d+>>     Shl [<<Param>>,<<Const16>>]
+  /// CHECK:     <<Shr:i\d+>>     Shr [<<Shl>>,<<Const16>>]
+  /// CHECK:     <<Conv:b\d+>>    TypeConversion [<<Shr>>]
+  /// CHECK:     <<Return:v\d+>>  Return [<<Conv>>]
+
+  /// CHECK-START: int Main.$noinline$testByteToShortToByte(byte) instruction_simplifier (after)
+  /// CHECK:     <<Param:b\d+>>   ParameterValue
+  /// CHECK:     <<Return:v\d+>>  Return [<<Param>>]
+
+  /// CHECK-START: int Main.$noinline$testByteToShortToByte(byte) instruction_simplifier (after)
+  /// CHECK-NOT:                  Shl
+
+  /// CHECK-START: int Main.$noinline$testByteToShortToByte(byte) instruction_simplifier (after)
+  /// CHECK-NOT:                  Shr
+
+  /// CHECK-START: int Main.$noinline$testByteToShortToByte(byte) instruction_simplifier (after)
+  /// CHECK-NOT:                  TypeConversion
+  private static int $noinline$testByteToShortToByte(byte arg) {
+    return (byte) ((arg << 16) >> 16);
+  }
+
+  /// CHECK-START: int Main.$noinline$testUnsignedByteToUnsignedByteToByte(byte) instruction_simplifier (before)
+  /// CHECK:     <<Param:b\d+>>    ParameterValue
+  /// CHECK:     <<Const255:i\d+>> IntConstant 255
+  /// CHECK:     <<Const24:i\d+>>  IntConstant 24
+  /// CHECK:     <<And:i\d+>>      And [<<Param>>,<<Const255>>]
+  /// CHECK:     <<Shl:i\d+>>      Shl [<<And>>,<<Const24>>]
+  /// CHECK:     <<UShr:i\d+>>     UShr [<<Shl>>,<<Const24>>]
+  /// CHECK:     <<Conv:b\d+>>     TypeConversion [<<UShr>>]
+  /// CHECK:     <<Return:v\d+>>   Return [<<Conv>>]
+
+  /// CHECK-START: int Main.$noinline$testUnsignedByteToUnsignedByteToByte(byte) instruction_simplifier (after)
+  /// CHECK:     <<Param:b\d+>>   ParameterValue
+  /// CHECK:     <<Return:v\d+>>  Return [<<Param>>]
+
+  /// CHECK-START: int Main.$noinline$testUnsignedByteToUnsignedByteToByte(byte) instruction_simplifier (after)
+  /// CHECK-NOT:                  Shl
+
+  /// CHECK-START: int Main.$noinline$testUnsignedByteToUnsignedByteToByte(byte) instruction_simplifier (after)
+  /// CHECK-NOT:                  Shr
+
+  /// CHECK-START: int Main.$noinline$testUnsignedByteToUnsignedByteToByte(byte) instruction_simplifier (after)
+  /// CHECK-NOT:                  TypeConversion
+  private static int $noinline$testUnsignedByteToUnsignedByteToByte(byte arg) {
+    return (byte) (((arg & 0xFF) << 24) >>> 24);
+  }
+
+  /// CHECK-START: int Main.$noinline$testUnsignedByteToShortToByte(byte) instruction_simplifier (before)
+  /// CHECK:     <<Param:b\d+>>    ParameterValue
+  /// CHECK:     <<Const255:i\d+>> IntConstant 255
+  /// CHECK:     <<Const16:i\d+>>  IntConstant 16
+  /// CHECK:     <<And:i\d+>>      And [<<Param>>,<<Const255>>]
+  /// CHECK:     <<Shl:i\d+>>      Shl [<<And>>,<<Const16>>]
+  /// CHECK:     <<Shr:i\d+>>      Shr [<<Shl>>,<<Const16>>]
+  /// CHECK:     <<Conv:b\d+>>     TypeConversion [<<Shr>>]
+  /// CHECK:     <<Return:v\d+>>   Return [<<Conv>>]
+
+  /// CHECK-START: int Main.$noinline$testUnsignedByteToShortToByte(byte) instruction_simplifier (after)
+  /// CHECK:     <<Param:b\d+>>   ParameterValue
+  /// CHECK:     <<Return:v\d+>>  Return [<<Param>>]
+
+  /// CHECK-START: int Main.$noinline$testUnsignedByteToShortToByte(byte) instruction_simplifier (after)
+  /// CHECK-NOT:                  Shl
+
+  /// CHECK-START: int Main.$noinline$testUnsignedByteToShortToByte(byte) instruction_simplifier (after)
+  /// CHECK-NOT:                  Shr
+
+  /// CHECK-START: int Main.$noinline$testUnsignedByteToShortToByte(byte) instruction_simplifier (after)
+  /// CHECK-NOT:                  TypeConversion
+  private static int $noinline$testUnsignedByteToShortToByte(byte arg) {
+    return (byte) (((arg & 0xFF) << 16) >> 16);
+  }
+
+  /// CHECK-START: int Main.$noinline$testUnsignedByteToCharToByte(byte) instruction_simplifier (before)
+  /// CHECK:     <<Param:b\d+>>    ParameterValue
+  /// CHECK:     <<Const255:i\d+>> IntConstant 255
+  /// CHECK:     <<Const16:i\d+>>  IntConstant 16
+  /// CHECK:     <<And:i\d+>>      And [<<Param>>,<<Const255>>]
+  /// CHECK:     <<Shl:i\d+>>      Shl [<<And>>,<<Const16>>]
+  /// CHECK:     <<UShr:i\d+>>     UShr [<<Shl>>,<<Const16>>]
+  /// CHECK:     <<Conv:b\d+>>     TypeConversion [<<UShr>>]
+  /// CHECK:     <<Return:v\d+>>   Return [<<Conv>>]
+
+  /// CHECK-START: int Main.$noinline$testUnsignedByteToCharToByte(byte) instruction_simplifier (after)
+  /// CHECK:     <<Param:b\d+>>   ParameterValue
+  /// CHECK:     <<Return:v\d+>>  Return [<<Param>>]
+
+  /// CHECK-START: int Main.$noinline$testUnsignedByteToCharToByte(byte) instruction_simplifier (after)
+  /// CHECK-NOT:                  Shl
+
+  /// CHECK-START: int Main.$noinline$testUnsignedByteToCharToByte(byte) instruction_simplifier (after)
+  /// CHECK-NOT:                  Shr
+
+  /// CHECK-START: int Main.$noinline$testUnsignedByteToCharToByte(byte) instruction_simplifier (after)
+  /// CHECK-NOT:                  TypeConversion
+  private static int $noinline$testUnsignedByteToCharToByte(byte arg) {
+    return (byte) (((arg & 0xFF) << 16) >>> 16);
+  }
+
+  /// CHECK-START: int Main.$noinline$testShortToShortToByte(short) instruction_simplifier (before)
+  /// CHECK:     <<Param:s\d+>>   ParameterValue
+  /// CHECK:     <<Const16:i\d+>> IntConstant 16
+  /// CHECK:     <<Shl:i\d+>>     Shl [<<Param>>,<<Const16>>]
+  /// CHECK:     <<Shr:i\d+>>     Shr [<<Shl>>,<<Const16>>]
+  /// CHECK:     <<Conv:b\d+>>    TypeConversion [<<Shr>>]
+  /// CHECK:     <<Return:v\d+>>  Return [<<Conv>>]
+
+  /// CHECK-START: int Main.$noinline$testShortToShortToByte(short) instruction_simplifier (after)
+  /// CHECK:     <<Param:s\d+>>   ParameterValue
+  /// CHECK:     <<Conv:b\d+>>    TypeConversion [<<Param>>]
+  /// CHECK:     <<Return:v\d+>>  Return [<<Conv>>]
+
+  /// CHECK-START: int Main.$noinline$testShortToShortToByte(short) instruction_simplifier (after)
+  /// CHECK-NOT:                  Shl
+
+  /// CHECK-START: int Main.$noinline$testShortToShortToByte(short) instruction_simplifier (after)
+  /// CHECK-NOT:                  Shr
+  private static int $noinline$testShortToShortToByte(short arg) {
+    return (byte) ((arg << 16) >> 16);
+  }
+
+  /// CHECK-START: int Main.$noinline$testCharToCharToByte(char) instruction_simplifier (before)
+  /// CHECK:     <<Param:c\d+>>   ParameterValue
+  /// CHECK:     <<Const16:i\d+>> IntConstant 16
+  /// CHECK:     <<Shl:i\d+>>     Shl [<<Param>>,<<Const16>>]
+  /// CHECK:     <<UShr:i\d+>>    UShr [<<Shl>>,<<Const16>>]
+  /// CHECK:     <<Conv:b\d+>>    TypeConversion [<<UShr>>]
+  /// CHECK:     <<Return:v\d+>>  Return [<<Conv>>]
+
+  /// CHECK-START: int Main.$noinline$testCharToCharToByte(char) instruction_simplifier (after)
+  /// CHECK:     <<Param:c\d+>>   ParameterValue
+  /// CHECK:     <<Conv:b\d+>>    TypeConversion [<<Param>>]
+  /// CHECK:     <<Return:v\d+>>  Return [<<Conv>>]
+
+  /// CHECK-START: int Main.$noinline$testCharToCharToByte(char) instruction_simplifier (after)
+  /// CHECK-NOT:                  Shl
+
+  /// CHECK-START: int Main.$noinline$testCharToCharToByte(char) instruction_simplifier (after)
+  /// CHECK-NOT:                  Shr
+  private static int $noinline$testCharToCharToByte(char arg) {
+    return (byte) ((arg << 16) >>> 16);
+  }
+
   public static void main(String[] args) throws Exception {
     Class smaliTests2 = Class.forName("SmaliTests2");
     Method $noinline$XorAllOnes = smaliTests2.getMethod("$noinline$XorAllOnes", int.class);
@@ -4107,6 +4307,27 @@ public class Main {
         $noinline$testUnsignedPromotionPatternWithDifferentShiftAmountConstants(0xaabbccdd));
     assertIntEquals(0xffffffee,
         $noinline$testSignedPromotionPatternWithDifferentShiftAmountConstants(0xaabbccdd));
+
+    assertIntEquals(0xffaa, $noinline$testByteToByteToChar((byte) 0xaa));
+    assertIntEquals(0x0a, $noinline$testByteToByteToChar((byte) 0x0a));
+
+    assertIntEquals(0x0a, $noinline$testByteToShortToByte((byte) 0x0a));
+    assertIntEquals(0xffffffaa, $noinline$testByteToShortToByte((byte) 0xaa));
+
+    assertIntEquals(0x0a, $noinline$testUnsignedByteToUnsignedByteToByte((byte) 0x0a));
+    assertIntEquals(0xffffffaa, $noinline$testUnsignedByteToUnsignedByteToByte((byte) 0xaa));
+
+    assertIntEquals(0x0a, $noinline$testUnsignedByteToShortToByte((byte) 0x0a));
+    assertIntEquals(0xffffffaa, $noinline$testUnsignedByteToShortToByte((byte) 0xaa));
+
+    assertIntEquals(0x0a, $noinline$testUnsignedByteToCharToByte((byte) 0x0a));
+    assertIntEquals(0xffffffaa, $noinline$testUnsignedByteToCharToByte((byte) 0xaa));
+
+    assertIntEquals(0x0b, $noinline$testShortToShortToByte((short) 0xaa0b));
+    assertIntEquals(0xffffffbb, $noinline$testShortToShortToByte((short) 0xaabb));
+
+    assertIntEquals(0x0b, $noinline$testCharToCharToByte((char) 0xaa0b));
+    assertIntEquals(0xffffffbb, $noinline$testCharToCharToByte((char) 0xaabb));
   }
 
   private static boolean $inline$true() { return true; }