ART: Fix the simplifier for add/sub

Instruction simplifier for add/sub should not proceed with floats
because that might cause the incorrect behavior with signed zero.

Change-Id: If0c9bf3931bcbf96b0814f8605a86997aea37145
Signed-off-by: Serguei Katkov <serguei.i.katkov@intel.com>

diff --git a/compiler/optimizing/instruction_simplifier.cc b/compiler/optimizing/instruction_simplifier.cc
index d391145..931a1c3 100644 (file)
--- a/compiler/optimizing/instruction_simplifier.cc
+++ b/compiler/optimizing/instruction_simplifier.cc
@@ -438,9 +438,14 @@ void InstructionSimplifierVisitor::VisitAdd(HAdd* instruction) {
     //    ADD dst, src, 0
     // with
     //    src
-    instruction->ReplaceWith(input_other);
-    instruction->GetBlock()->RemoveInstruction(instruction);
-    return;
+    // Note that we cannot optimize `x + 0.0` to `x` for floating-point. When
+    // `x` is `-0.0`, the former expression yields `0.0`, while the later
+    // yields `-0.0`.
+    if (Primitive::IsIntegralType(instruction->GetType())) {
+      instruction->ReplaceWith(input_other);
+      instruction->GetBlock()->RemoveInstruction(instruction);
+      return;
+    }
   }
 
   HInstruction* left = instruction->GetLeft();
@@ -800,21 +805,24 @@ void InstructionSimplifierVisitor::VisitSub(HSub* instruction) {
   HConstant* input_cst = instruction->GetConstantRight();
   HInstruction* input_other = instruction->GetLeastConstantLeft();
 
+  Primitive::Type type = instruction->GetType();
+  if (Primitive::IsFloatingPointType(type)) {
+    return;
+  }
+
   if ((input_cst != nullptr) && input_cst->IsZero()) {
     // Replace code looking like
     //    SUB dst, src, 0
     // with
     //    src
+    // Note that we cannot optimize `x - 0.0` to `x` for floating-point. When
+    // `x` is `-0.0`, the former expression yields `0.0`, while the later
+    // yields `-0.0`.
     instruction->ReplaceWith(input_other);
     instruction->GetBlock()->RemoveInstruction(instruction);
     return;
   }
 
-  Primitive::Type type = instruction->GetType();
-  if (!Primitive::IsIntegralType(type)) {
-    return;
-  }
-
   HBasicBlock* block = instruction->GetBlock();
   ArenaAllocator* allocator = GetGraph()->GetArena();
 

diff --git a/test/474-fp-sub-neg/expected.txt b/test/474-fp-sub-neg/expected.txt
index e6ffe0d..1c15abb 100644 (file)
--- a/test/474-fp-sub-neg/expected.txt
+++ b/test/474-fp-sub-neg/expected.txt
@@ -1,2 +1,6 @@
 -0.0
+0.0
+0.0
 -0.0
+0.0
+0.0

diff --git a/test/474-fp-sub-neg/info.txt b/test/474-fp-sub-neg/info.txt
index eced93f..82effdb 100644 (file)
--- a/test/474-fp-sub-neg/info.txt
+++ b/test/474-fp-sub-neg/info.txt
@@ -1,5 +1,11 @@
 Regression check for optimizing simplify instruction pass.
+
 A pair (sub, neg) should not be transforemd to (sub) for
 fp calculation because we can lose the sign of zero for
 the following expression:
   - ( A - B ) != B - A ; if B == A
+
+Addition or subtraction with fp zero should not be eliminated
+because:
+  -0.0 + 0.0 = 0.0
+  -0.0 - -0.0 = 0.0

diff --git a/test/474-fp-sub-neg/src/Main.java b/test/474-fp-sub-neg/src/Main.java
index e6bce67..c190e8e 100644 (file)
--- a/test/474-fp-sub-neg/src/Main.java
+++ b/test/474-fp-sub-neg/src/Main.java
@@ -24,6 +24,8 @@ public class Main {
       }
 
       System.out.println(f);
+      System.out.println(f + 0f);
+      System.out.println(f - (-0f));
     }
 
     public static void doubleTest() {
@@ -35,6 +37,8 @@ public class Main {
       }
 
       System.out.println(d);
+      System.out.println(d + 0f);
+      System.out.println(d - (-0f));
     }
 
     public static void main(String[] args) {