Optimize the multiplication of two 32-bit unsigned integers.

runtime/vm/intermediate_language_ia32.cc

Index: runtime/vm/intermediate_language_ia32.cc
===================================================================
--- runtime/vm/intermediate_language_ia32.cc	(revision 38266)
+++ runtime/vm/intermediate_language_ia32.cc	(working copy)
@@ -651,7 +651,7 @@
   Register cid_reg = locs()->temp(0).reg();
 
   Label* deopt = CanDeoptimize() ?
-  compiler->AddDeoptStub(deopt_id(), ICData::kDeoptTestCids) : NULL;
+      compiler->AddDeoptStub(deopt_id(), ICData::kDeoptTestCids) : NULL;
 
   const intptr_t true_result = (kind() == Token::kIS) ? 1 : 0;
   const ZoneGrowableArray<intptr_t>& data = cid_results();
@@ -5846,7 +5846,9 @@
   switch (op_kind()) {
     case Token::kBIT_AND:
     case Token::kBIT_OR:
-    case Token::kBIT_XOR: {
+    case Token::kBIT_XOR:
+    case Token::kADD:
+    case Token::kSUB: {
       const intptr_t kNumTemps = 0;
       LocationSummary* summary = new(isolate) LocationSummary(
           isolate, kNumInputs, kNumTemps, LocationSummary::kNoCall);
@@ -5857,16 +5859,16 @@
       summary->set_out(0, Location::SameAsFirstInput());
       return summary;
     }
-    case Token::kADD:
-    case Token::kSUB: {
+    case Token::kMUL: {
       const intptr_t kNumTemps = 0;
       LocationSummary* summary = new(isolate) LocationSummary(
           isolate, kNumInputs, kNumTemps, LocationSummary::kNoCall);
-      summary->set_in(0, Location::Pair(Location::RequiresRegister(),
-                                        Location::RequiresRegister()));
+      summary->set_in(0, Location::Pair(Location::RegisterLocation(EAX),
+                                        Location::RegisterLocation(EDX)));
       summary->set_in(1, Location::Pair(Location::RequiresRegister(),
                                         Location::RequiresRegister()));
-      summary->set_out(0, Location::SameAsFirstInput());
+      summary->set_out(0, Location::Pair(Location::RegisterLocation(EAX),
+                                         Location::RegisterLocation(EDX)));
       return summary;
     }
     default:
@@ -5920,6 +5922,23 @@
       }
       break;
     }
+    case Token::kMUL: {
+      // We only support the multiplication of two positive 32-bit integers
+      // resulting in a positive 64-bit integer fitting in a mint.
+      // We deopt in all other cases.
+      // This guarantees that the multiplication of 16-bit unsigned integers,
+      // as used in bignum arithmetic, will always succeed.
+      __ orl(left_hi, right_hi);
+      __ j(NOT_ZERO, deopt);
+      ASSERT(left_lo == EAX);
+      __ mull(right_lo);  // Result in EDX:EAX.
+      if (can_overflow()) {
+        __ testl(right_hi, Immediate(0xc0000000));
+        __ shrl(right_hi, Immediate(30));

[sra1, 2014/07/16 00:58:27]

Maybe I misunderstand, but...

Why test right_hi? Surely it is zero from the orl/jnz test?
The product is in EDX:EAX which is aliased to left_hi:left_lo and out_hi:out_lo.

Should shrl be removed?
The flags generated by testl flags are killed by shrl, and doesn't shrl kill the
high word of the result?

[regis, 2014/07/16 02:23:22]

Oops, the shrl is left over from a previous version and is redundant here. Yes,
it should be removed.

Also, it should be out_hi, not right_hi. I should have tested with
--unbox_mints=false

out_hi is guaranteed to be EDX and it contains the upper 32-bit of the result.
I've added asserts.

Thanks!

+        __ j(NOT_ZERO, deopt);
+      }
+      break;
+    }
     default: UNREACHABLE();
   }
   if (FLAG_throw_on_javascript_int_overflow) {
@@ -6193,10 +6212,15 @@
 LocationSummary* BinaryUint32OpInstr::MakeLocationSummary(Isolate* isolate,
                                                           bool opt) const {
   const intptr_t kNumInputs = 2;
-  const intptr_t kNumTemps = 0;
+  const intptr_t kNumTemps = (op_kind() == Token::kMUL) ? 1 : 0;
   LocationSummary* summary = new(isolate) LocationSummary(
       isolate, kNumInputs, kNumTemps, LocationSummary::kNoCall);
-  summary->set_in(0, Location::RequiresRegister());
+  if (op_kind() == Token::kMUL) {
+    summary->set_in(0, Location::RegisterLocation(EAX));
+    summary->set_temp(0, Location::RegisterLocation(EDX));
+  } else {
+    summary->set_in(0, Location::RequiresRegister());
+  }
   summary->set_in(1, Location::RequiresRegister());
   summary->set_out(0, Location::SameAsFirstInput());
   return summary;
@@ -6208,22 +6232,28 @@
   Register right = locs()->in(1).reg();
   Register out = locs()->out(0).reg();
   ASSERT(out == left);
+  Label* deopt = CanDeoptimize() ?
+      compiler->AddDeoptStub(deopt_id(), ICData::kDeoptBinaryUint32Op) : NULL;
   switch (op_kind()) {
     case Token::kBIT_AND:
       __ andl(out, right);
-    break;
+      break;
     case Token::kBIT_OR:
       __ orl(out, right);
-    break;
+      break;
     case Token::kBIT_XOR:
       __ xorl(out, right);
-    break;
+      break;
     case Token::kADD:
       __ addl(out, right);
-    break;
+      break;
     case Token::kSUB:
       __ subl(out, right);
-    break;
+      break;
+    case Token::kMUL:
+      __ mull(right);  // Result in EDX:EAX, CF set if EDX != 0.
+      __ j(OVERFLOW, deopt);
+      break;
     default:
       UNREACHABLE();
   }