From 625090fe9bf47d8d735c9a66cbf491de3a3e3765 Mon Sep 17 00:00:00 2001 From: Vladimir Marko Date: Mon, 14 Mar 2016 18:00:05 +0000 Subject: [PATCH] Optimizing: Fix TypeConversion(And(x, const)) simplification. Avoid introducing implicit conversions when simplifying the expression TypeConversion(And(x, const)). Previously, when we dropped the And, we could end up with a TypeConversion to the same type which should be eliminated on subsequent pass of the block's instructions; however, a subsequent dependent TypeConversion in the same block would be processed earlier and we would unexpectedly see its input as the conversion to the same type, failing a DCHECK(). Bug: 27626509 Change-Id: I5874a9ceafbf635cf3391beea807ede8468ab5c3 --- compiler/optimizing/instruction_simplifier.cc | 20 ++-- .../src/Main.java | 125 ++++++++++++--------- 2 files changed, 88 insertions(+), 57 deletions(-) diff --git a/compiler/optimizing/instruction_simplifier.cc b/compiler/optimizing/instruction_simplifier.cc index 049901b88..69dad697a 100644 --- a/compiler/optimizing/instruction_simplifier.cc +++ b/compiler/optimizing/instruction_simplifier.cc @@ -867,9 +867,7 @@ void InstructionSimplifierVisitor::VisitTypeConversion(HTypeConversion* instruct return; } } - } else if (input->IsAnd() && - Primitive::IsIntegralType(result_type) && - input->HasOnlyOneNonEnvironmentUse()) { + } else if (input->IsAnd() && Primitive::IsIntegralType(result_type)) { DCHECK(Primitive::IsIntegralType(input_type)); HAnd* input_and = input->AsAnd(); HConstant* constant = input_and->GetConstantRight(); @@ -879,10 +877,18 @@ void InstructionSimplifierVisitor::VisitTypeConversion(HTypeConversion* instruct size_t trailing_ones = CTZ(~static_cast(value)); if (trailing_ones >= kBitsPerByte * Primitive::ComponentSize(result_type)) { // The `HAnd` is useless, for example in `(byte) (x & 0xff)`, get rid of it. - input_and->ReplaceWith(input_and->GetLeastConstantLeft()); - input_and->GetBlock()->RemoveInstruction(input_and); - RecordSimplification(); - return; + HInstruction* original_input = input_and->GetLeastConstantLeft(); + if (IsTypeConversionImplicit(original_input->GetType(), result_type)) { + instruction->ReplaceWith(original_input); + instruction->GetBlock()->RemoveInstruction(instruction); + RecordSimplification(); + return; + } else if (input->HasOnlyOneNonEnvironmentUse()) { + input_and->ReplaceWith(original_input); + input_and->GetBlock()->RemoveInstruction(input_and); + RecordSimplification(); + return; + } } } } diff --git a/test/458-checker-instruction-simplification/src/Main.java b/test/458-checker-instruction-simplification/src/Main.java index dd4ffe45e..f7c721a19 100644 --- a/test/458-checker-instruction-simplification/src/Main.java +++ b/test/458-checker-instruction-simplification/src/Main.java @@ -1601,6 +1601,24 @@ public class Main { return (short) (value & 0x17fff); } + /// CHECK-START: double Main.shortAnd0xffffToShortToDouble(short) instruction_simplifier (before) + /// CHECK-DAG: <> ParameterValue + /// CHECK-DAG: <> IntConstant 65535 + /// CHECK-DAG: <> And [<>,<>] + /// CHECK-DAG: <> TypeConversion [<>] + /// CHECK-DAG: <> TypeConversion [<>] + /// CHECK-DAG: Return [<>] + + /// CHECK-START: double Main.shortAnd0xffffToShortToDouble(short) instruction_simplifier (after) + /// CHECK-DAG: <> ParameterValue + /// CHECK-DAG: <> TypeConversion [<>] + /// CHECK-DAG: Return [<>] + + public static double shortAnd0xffffToShortToDouble(short value) { + short same = (short) (value & 0xffff); + return (double) same; + } + /// CHECK-START: int Main.intReverseCondition(int) instruction_simplifier (before) /// CHECK-DAG: <> ParameterValue /// CHECK-DAG: <> IntConstant 42 @@ -1717,56 +1735,63 @@ public static void main(String[] args) { assertIntEquals(doubleConditionEqualZero(6.0), 54); assertIntEquals(doubleConditionEqualZero(43.0), 13); - assertIntEquals(intToDoubleToInt(1234567), 1234567); - assertIntEquals(intToDoubleToInt(Integer.MIN_VALUE), Integer.MIN_VALUE); - assertIntEquals(intToDoubleToInt(Integer.MAX_VALUE), Integer.MAX_VALUE); - assertStringEquals(intToDoubleToIntPrint(7654321), "d=7654321.0, i=7654321"); - assertIntEquals(byteToDoubleToInt((byte) 12), 12); - assertIntEquals(byteToDoubleToInt(Byte.MIN_VALUE), Byte.MIN_VALUE); - assertIntEquals(byteToDoubleToInt(Byte.MAX_VALUE), Byte.MAX_VALUE); - assertIntEquals(floatToDoubleToInt(11.3f), 11); - assertStringEquals(floatToDoubleToIntPrint(12.25f), "d=12.25, i=12"); - assertIntEquals(byteToDoubleToShort((byte) 123), 123); - assertIntEquals(byteToDoubleToShort(Byte.MIN_VALUE), Byte.MIN_VALUE); - assertIntEquals(byteToDoubleToShort(Byte.MAX_VALUE), Byte.MAX_VALUE); - assertIntEquals(charToDoubleToShort((char) 1234), 1234); - assertIntEquals(charToDoubleToShort(Character.MIN_VALUE), Character.MIN_VALUE); - assertIntEquals(charToDoubleToShort(Character.MAX_VALUE), /* sign-extended */ -1); - assertIntEquals(floatToIntToShort(12345.75f), 12345); - assertIntEquals(floatToIntToShort((float)(Short.MIN_VALUE - 1)), Short.MAX_VALUE); - assertIntEquals(floatToIntToShort((float)(Short.MAX_VALUE + 1)), Short.MIN_VALUE); - assertIntEquals(intToFloatToInt(-54321), -54321); - assertDoubleEquals(longToIntToDouble(0x1234567812345678L), (double) 0x12345678); - assertDoubleEquals(longToIntToDouble(Long.MIN_VALUE), 0.0); - assertDoubleEquals(longToIntToDouble(Long.MAX_VALUE), -1.0); - assertLongEquals(longToIntToLong(0x1234567812345678L), 0x0000000012345678L); - assertLongEquals(longToIntToLong(0x1234567887654321L), 0xffffffff87654321L); - assertLongEquals(longToIntToLong(Long.MIN_VALUE), 0L); - assertLongEquals(longToIntToLong(Long.MAX_VALUE), -1L); - assertIntEquals(shortToCharToShort((short) -5678), (short) -5678); - assertIntEquals(shortToCharToShort(Short.MIN_VALUE), Short.MIN_VALUE); - assertIntEquals(shortToCharToShort(Short.MAX_VALUE), Short.MAX_VALUE); - assertIntEquals(shortToLongToInt((short) 5678), 5678); - assertIntEquals(shortToLongToInt(Short.MIN_VALUE), Short.MIN_VALUE); - assertIntEquals(shortToLongToInt(Short.MAX_VALUE), Short.MAX_VALUE); - assertIntEquals(shortToCharToByte((short) 0x1234), 0x34); - assertIntEquals(shortToCharToByte((short) 0x12f0), -0x10); - assertIntEquals(shortToCharToByte(Short.MIN_VALUE), 0); - assertIntEquals(shortToCharToByte(Short.MAX_VALUE), -1); - assertStringEquals(shortToCharToBytePrint((short) 1025), "c=1025, b=1"); - assertStringEquals(shortToCharToBytePrint((short) 1023), "c=1023, b=-1"); - assertStringEquals(shortToCharToBytePrint((short) -1), "c=65535, b=-1"); - - assertIntEquals(longAnd0xffToByte(0x1234432112344321L), 0x21); - assertIntEquals(longAnd0xffToByte(Long.MIN_VALUE), 0); - assertIntEquals(longAnd0xffToByte(Long.MAX_VALUE), -1); - assertIntEquals(intAnd0x1ffffToChar(0x43211234), 0x1234); - assertIntEquals(intAnd0x1ffffToChar(Integer.MIN_VALUE), 0); - assertIntEquals(intAnd0x1ffffToChar(Integer.MAX_VALUE), Character.MAX_VALUE); - assertIntEquals(intAnd0x17fffToShort(0x87654321), 0x4321); - assertIntEquals(intAnd0x17fffToShort(0x88888888), 0x0888); - assertIntEquals(intAnd0x17fffToShort(Integer.MIN_VALUE), 0); - assertIntEquals(intAnd0x17fffToShort(Integer.MAX_VALUE), Short.MAX_VALUE); + assertIntEquals(1234567, intToDoubleToInt(1234567)); + assertIntEquals(Integer.MIN_VALUE, intToDoubleToInt(Integer.MIN_VALUE)); + assertIntEquals(Integer.MAX_VALUE, intToDoubleToInt(Integer.MAX_VALUE)); + assertStringEquals("d=7654321.0, i=7654321", intToDoubleToIntPrint(7654321)); + assertIntEquals(12, byteToDoubleToInt((byte) 12)); + assertIntEquals(Byte.MIN_VALUE, byteToDoubleToInt(Byte.MIN_VALUE)); + assertIntEquals(Byte.MAX_VALUE, byteToDoubleToInt(Byte.MAX_VALUE)); + assertIntEquals(11, floatToDoubleToInt(11.3f)); + assertStringEquals("d=12.25, i=12", floatToDoubleToIntPrint(12.25f)); + assertIntEquals(123, byteToDoubleToShort((byte) 123)); + assertIntEquals(Byte.MIN_VALUE, byteToDoubleToShort(Byte.MIN_VALUE)); + assertIntEquals(Byte.MAX_VALUE, byteToDoubleToShort(Byte.MAX_VALUE)); + assertIntEquals(1234, charToDoubleToShort((char) 1234)); + assertIntEquals(Character.MIN_VALUE, charToDoubleToShort(Character.MIN_VALUE)); + assertIntEquals(/* sign-extended */ -1, charToDoubleToShort(Character.MAX_VALUE)); + assertIntEquals(12345, floatToIntToShort(12345.75f)); + assertIntEquals(Short.MAX_VALUE, floatToIntToShort((float)(Short.MIN_VALUE - 1))); + assertIntEquals(Short.MIN_VALUE, floatToIntToShort((float)(Short.MAX_VALUE + 1))); + assertIntEquals(-54321, intToFloatToInt(-54321)); + assertDoubleEquals((double) 0x12345678, longToIntToDouble(0x1234567812345678L)); + assertDoubleEquals(0.0, longToIntToDouble(Long.MIN_VALUE)); + assertDoubleEquals(-1.0, longToIntToDouble(Long.MAX_VALUE)); + assertLongEquals(0x0000000012345678L, longToIntToLong(0x1234567812345678L)); + assertLongEquals(0xffffffff87654321L, longToIntToLong(0x1234567887654321L)); + assertLongEquals(0L, longToIntToLong(Long.MIN_VALUE)); + assertLongEquals(-1L, longToIntToLong(Long.MAX_VALUE)); + assertIntEquals((short) -5678, shortToCharToShort((short) -5678)); + assertIntEquals(Short.MIN_VALUE, shortToCharToShort(Short.MIN_VALUE)); + assertIntEquals(Short.MAX_VALUE, shortToCharToShort(Short.MAX_VALUE)); + assertIntEquals(5678, shortToLongToInt((short) 5678)); + assertIntEquals(Short.MIN_VALUE, shortToLongToInt(Short.MIN_VALUE)); + assertIntEquals(Short.MAX_VALUE, shortToLongToInt(Short.MAX_VALUE)); + assertIntEquals(0x34, shortToCharToByte((short) 0x1234)); + assertIntEquals(-0x10, shortToCharToByte((short) 0x12f0)); + assertIntEquals(0, shortToCharToByte(Short.MIN_VALUE)); + assertIntEquals(-1, shortToCharToByte(Short.MAX_VALUE)); + assertStringEquals("c=1025, b=1", shortToCharToBytePrint((short) 1025)); + assertStringEquals("c=1023, b=-1", shortToCharToBytePrint((short) 1023)); + assertStringEquals("c=65535, b=-1", shortToCharToBytePrint((short) -1)); + + assertIntEquals(0x21, longAnd0xffToByte(0x1234432112344321L)); + assertIntEquals(0, longAnd0xffToByte(Long.MIN_VALUE)); + assertIntEquals(-1, longAnd0xffToByte(Long.MAX_VALUE)); + assertIntEquals(0x1234, intAnd0x1ffffToChar(0x43211234)); + assertIntEquals(0, intAnd0x1ffffToChar(Integer.MIN_VALUE)); + assertIntEquals(Character.MAX_VALUE, intAnd0x1ffffToChar(Integer.MAX_VALUE)); + assertIntEquals(0x4321, intAnd0x17fffToShort(0x87654321)); + assertIntEquals(0x0888, intAnd0x17fffToShort(0x88888888)); + assertIntEquals(0, intAnd0x17fffToShort(Integer.MIN_VALUE)); + assertIntEquals(Short.MAX_VALUE, intAnd0x17fffToShort(Integer.MAX_VALUE)); + + assertDoubleEquals(0.0, shortAnd0xffffToShortToDouble((short) 0)); + assertDoubleEquals(1.0, shortAnd0xffffToShortToDouble((short) 1)); + assertDoubleEquals(-2.0, shortAnd0xffffToShortToDouble((short) -2)); + assertDoubleEquals(12345.0, shortAnd0xffffToShortToDouble((short) 12345)); + assertDoubleEquals((double)Short.MAX_VALUE, shortAnd0xffffToShortToDouble(Short.MAX_VALUE)); + assertDoubleEquals((double)Short.MIN_VALUE, shortAnd0xffffToShortToDouble(Short.MIN_VALUE)); assertIntEquals(intReverseCondition(41), 13); assertIntEquals(intReverseConditionNaN(-5), 13); -- 2.11.0