Revise assertions and fix bug in the implementation of shifts

runtime/vm/intermediate_language_x64.cc

 // Copyright (c) 2013, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 #include "vm/globals.h"  // Needed here to get TARGET_ARCH_X64.
 #if defined(TARGET_ARCH_X64)
 
 #include "vm/intermediate_language.h"
 
 #include "vm/compiler.h"
@@ skipping 6046 matching lines @@
   return summary;
 }
 
 
 void ShiftMintOpInstr::EmitNativeCode(FlowGraphCompiler* compiler) {
   const Register left = locs()->in(0).reg();
   const Register out = locs()->out(0).reg();
   ASSERT(left == out);
 
   Label* deopt = NULL;
   if (CanDeoptimize()) {

[regis, 2017/07/11 18:45:55]

When specifying --limit-ints-to-64-bits, we probably do not want to deoptimize,
but instead let the cpu do the operation, no? The behavior may not be the same
on all architectures, though. That is something to discuss and definitely not
for this CL. A TODO may be a good idea.

[alexmarkov, 2017/07/11 19:39:23]

Acknowledged.

     deopt = compiler->AddDeoptStub(deopt_id(), ICData::kDeoptBinaryMintOp);
   }
   if (locs()->in(1).IsConstant()) {
     // Code for a constant shift amount.
     ASSERT(locs()->in(1).constant().IsSmi());
     const int64_t shift =
         reinterpret_cast<int64_t>(locs()->in(1).constant().raw()) >> 1;
-    // TODO(alexmarkov): revise and uncomment the following assertions
-    // ASSERT(!has_shift_count_check());
-    // ASSERT((0 <= shift) && (shift < 64));
+    ASSERT(shift >= 0);
     switch (op_kind()) {
       case Token::kSHR:
-        __ sarq(left, Immediate(shift));
+        __ sarq(left,
+                Immediate(Utils::Minimum<int64_t>(shift, kBitsPerWord - 1)));
         break;
       case Token::kSHL: {
+        ASSERT(shift < 64);
         if (can_overflow()) {
           // Check for overflow.
           Register temp = locs()->temp(0).reg();
           __ movq(temp, left);
           __ shlq(left, Immediate(shift));
           __ sarq(left, Immediate(shift));
           __ cmpq(left, temp);
           __ j(NOT_EQUAL, deopt);  // Overflow.
         }
         // Shift for result now we know there is no overflow.
         __ shlq(left, Immediate(shift));
         break;
       }
       default:
         UNREACHABLE();
     }
   } else {
     // Code for a variable shift amount.
     // Deoptimize if shift count is > 63.
     // sarl operation masks the count to 5 bits and
     // shrd is undefined with count > operand size (32)

[regis, 2017/07/11 18:45:55]

This last comment does not look correct? shrd with a 64-bit destination
considers 6 bits of the count.

[alexmarkov, 2017/07/11 19:39:23]

Done.

     __ SmiUntag(RCX);
-    if (has_shift_count_check()) {
+    if (!IsShiftCountInRange()) {
       __ cmpq(RCX, Immediate(kMintShiftCountLimit));
       __ j(ABOVE, deopt);
     }
     Label done, large_shift;
     switch (op_kind()) {
       case Token::kSHR: {
         __ sarq(left, RCX);
         break;
       }
       case Token::kSHL: {
@@ skipping 533 matching lines @@
   compiler->AddCurrentDescriptor(stub_kind_, deopt_id_, token_pos());
   compiler->RecordSafepoint(locs());
 }
 
 
 }  // namespace dart
 
 #undef __
 
 #endif  // defined TARGET_ARCH_X64