[turbofan][csa] optimize Smi untagging better

- Introduce new operator variants for signed right-shifts with the
  additional information that they always shift out zeros.
- Use these new operators for Smi untagging.
- Merge left-shifts with a preceding Smi-untagging shift.
- Optimize comparisons of Smi-untagging shifts to operate on the
  unshifted word.
- Optimize 64bit comparisons of values expanded from 32bit to use
  a 32bit comparison instead.
- Change CodeStubAssembler::UntagSmi to first sign-extend and then
  right-shift to enable better address computations for Smi indices.

Bug: v8:9962
Change-Id: If91300f365e8f01457aebf0bd43bdf88b305c460
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2135734
Commit-Queue: Tobias Tebbi <[email protected]>
Reviewed-by: Georg Neis <[email protected]>
Cr-Commit-Position: refs/heads/master@{#67378}

src/codegen/code-stub-assembler.cc

@@ -763,8 +763,8 @@ TNode<BoolT> CodeStubAssembler::IsValidSmiIndex(TNode<Smi> smi) {
 
 TNode<IntPtrT> CodeStubAssembler::TaggedIndexToIntPtr(
     TNode<TaggedIndex> value) {
-  return Signed(WordSar(BitcastTaggedToWordForTagAndSmiBits(value),
-                        IntPtrConstant(kSmiTagSize)));
+  return Signed(WordSarShiftOutZeros(BitcastTaggedToWordForTagAndSmiBits(value),
+                                     IntPtrConstant(kSmiTagSize)));
 }
 
 TNode<TaggedIndex> CodeStubAssembler::IntPtrToTaggedIndex(
@@ -859,16 +859,17 @@ TNode<IntPtrT> CodeStubAssembler::SmiUntag(SloppyTNode<Smi> value) {
   if (ToIntPtrConstant(value, &constant_value)) {
     return IntPtrConstant(constant_value >> (kSmiShiftSize + kSmiTagSize));
   }
+  TNode<IntPtrT> raw_bits = BitcastTaggedToWordForTagAndSmiBits(value);
   if (COMPRESS_POINTERS_BOOL) {
-    return ChangeInt32ToIntPtr(SmiToInt32(value));
+    // Clear the upper half using sign-extension.
+    raw_bits = ChangeInt32ToIntPtr(TruncateIntPtrToInt32(raw_bits));
   }
-  return Signed(WordSar(BitcastTaggedToWordForTagAndSmiBits(value),
-                        SmiShiftBitsConstant()));
+  return Signed(WordSarShiftOutZeros(raw_bits, SmiShiftBitsConstant()));
 }
 
 TNode<Int32T> CodeStubAssembler::SmiToInt32(SloppyTNode<Smi> value) {
   if (COMPRESS_POINTERS_BOOL) {
-    return Signed(Word32Sar(
+    return Signed(Word32SarShiftOutZeros(
         TruncateIntPtrToInt32(BitcastTaggedToWordForTagAndSmiBits(value)),
         SmiShiftBitsConstant32()));
   }

src/compiler/backend/instruction-selector.cc

@@ -12,6 +12,7 @@
 #include "src/compiler/backend/instruction-selector-impl.h"
 #include "src/compiler/compiler-source-position-table.h"
 #include "src/compiler/node-matchers.h"
+#include "src/compiler/node-properties.h"
 #include "src/compiler/pipeline.h"
 #include "src/compiler/schedule.h"
 #include "src/compiler/state-values-utils.h"
@@ -3014,7 +3015,8 @@ void InstructionSelector::VisitUnreachable(Node* node) {
 }
 
 void InstructionSelector::VisitStaticAssert(Node* node) {
-  node->InputAt(0)->Print();
+  Node* asserted = node->InputAt(0);
+  asserted->Print(2);
   FATAL("Expected turbofan static assert to hold, but got non-true input!\n");
 }
 

src/compiler/code-assembler.h

@@ -273,13 +273,15 @@ class CodeAssemblerParameterizedLabel;
   V(WordShl, WordT, WordT, IntegralT)                                   \
   V(WordShr, WordT, WordT, IntegralT)                                   \
   V(WordSar, WordT, WordT, IntegralT)                                   \
+  V(WordSarShiftOutZeros, WordT, WordT, IntegralT)                      \
   V(Word32Or, Word32T, Word32T, Word32T)                                \
   V(Word32And, Word32T, Word32T, Word32T)                               \
   V(Word32Xor, Word32T, Word32T, Word32T)                               \
   V(Word32Ror, Word32T, Word32T, Word32T)                               \
   V(Word32Shl, Word32T, Word32T, Word32T)                               \
   V(Word32Shr, Word32T, Word32T, Word32T)                               \
   V(Word32Sar, Word32T, Word32T, Word32T)                               \
+  V(Word32SarShiftOutZeros, Word32T, Word32T, Word32T)                  \
   V(Word64And, Word64T, Word64T, Word64T)                               \
   V(Word64Or, Word64T, Word64T, Word64T)                                \
   V(Word64Xor, Word64T, Word64T, Word64T)                               \

src/compiler/effect-control-linearizer.cc

@@ -4552,18 +4552,20 @@ Node* EffectControlLinearizer::ChangeUint32ToSmi(Node* value) {
 }
 
 Node* EffectControlLinearizer::ChangeSmiToIntPtr(Node* value) {
-  // Do shift on 32bit values if Smis are stored in the lower word.
   if (machine()->Is64() && SmiValuesAre31Bits()) {
-    return __ ChangeInt32ToInt64(
-        __ Word32Sar(__ TruncateInt64ToInt32(value), SmiShiftBitsConstant()));
+    // First sign-extend the upper half, then shift away the Smi tag.
+    return __ WordSarShiftOutZeros(
+        __ ChangeInt32ToInt64(__ TruncateInt64ToInt32(value)),
+        SmiShiftBitsConstant());
   }
-  return __ WordSar(value, SmiShiftBitsConstant());
+  return __ WordSarShiftOutZeros(value, SmiShiftBitsConstant());
 }
 
 Node* EffectControlLinearizer::ChangeSmiToInt32(Node* value) {
   // Do shift on 32bit values if Smis are stored in the lower word.
   if (machine()->Is64() && SmiValuesAre31Bits()) {
-    return __ Word32Sar(__ TruncateInt64ToInt32(value), SmiShiftBitsConstant());
+    return __ Word32SarShiftOutZeros(__ TruncateInt64ToInt32(value),
+                                     SmiShiftBitsConstant());
   }
   if (machine()->Is64()) {
     return __ TruncateInt64ToInt32(ChangeSmiToIntPtr(value));

src/compiler/graph-assembler.h

@@ -82,6 +82,7 @@ class BasicBlock;
   V(Word32Equal)                          \
   V(Word32Or)                             \
   V(Word32Sar)                            \
+  V(Word32SarShiftOutZeros)               \
   V(Word32Shl)                            \
   V(Word32Shr)                            \
   V(Word32Xor)                            \
@@ -91,6 +92,7 @@ class BasicBlock;
   V(WordAnd)                              \
   V(WordEqual)                            \
   V(WordSar)                              \
+  V(WordSarShiftOutZeros)                 \
   V(WordShl)
 
 #define CHECKED_ASSEMBLER_MACH_BINOP_LIST(V) \

src/compiler/machine-operator-reducer.cc

@@ -4,6 +4,7 @@
 
 #include "src/compiler/machine-operator-reducer.h"
 #include <cmath>
+#include <limits>
 
 #include "src/base/bits.h"
 #include "src/base/division-by-constant.h"
@@ -14,6 +15,7 @@
 #include "src/compiler/machine-graph.h"
 #include "src/compiler/node-matchers.h"
 #include "src/compiler/node-properties.h"
+#include "src/compiler/opcodes.h"
 #include "src/numbers/conversions-inl.h"
 
 namespace v8 {
@@ -423,15 +425,15 @@ Reduction MachineOperatorReducer::Reduce(Node* node) {
           return ReplaceBool(true);
         }
       }
-      break;
+      return ReduceWord32Comparisons(node);
     }
     case IrOpcode::kInt32LessThanOrEqual: {
       Int32BinopMatcher m(node);
       if (m.IsFoldable()) {  // K <= K => K
         return ReplaceBool(m.left().Value() <= m.right().Value());
       }
       if (m.LeftEqualsRight()) return ReplaceBool(true);  // x <= x => true
-      break;
+      return ReduceWord32Comparisons(node);
     }
     case IrOpcode::kUint32LessThan: {
       Uint32BinopMatcher m(node);
@@ -456,7 +458,7 @@ Reduction MachineOperatorReducer::Reduce(Node* node) {
           // TODO(turbofan): else the comparison is always true.
         }
       }
-      break;
+      return ReduceWord32Comparisons(node);
     }
     case IrOpcode::kUint32LessThanOrEqual: {
       Uint32BinopMatcher m(node);
@@ -466,7 +468,7 @@ Reduction MachineOperatorReducer::Reduce(Node* node) {
         return ReplaceBool(m.left().Value() <= m.right().Value());
       }
       if (m.LeftEqualsRight()) return ReplaceBool(true);  // x <= x => true
-      break;
+      return ReduceWord32Comparisons(node);
     }
     case IrOpcode::kFloat32Sub: {
       Float32BinopMatcher m(node);
@@ -873,6 +875,11 @@ Reduction MachineOperatorReducer::Reduce(Node* node) {
     case IrOpcode::kTrapIf:
     case IrOpcode::kTrapUnless:
       return ReduceConditional(node);
+    case IrOpcode::kInt64LessThan:
+    case IrOpcode::kInt64LessThanOrEqual:
+    case IrOpcode::kUint64LessThan:
+    case IrOpcode::kUint64LessThanOrEqual:
+      return ReduceWord64Comparisons(node);
     default:
       break;
   }
@@ -1247,6 +1254,78 @@ Reduction MachineOperatorReducer::ReduceProjection(size_t index, Node* node) {
   return NoChange();
 }
 
+Reduction MachineOperatorReducer::ReduceWord32Comparisons(Node* node) {
+  DCHECK(node->opcode() == IrOpcode::kInt32LessThan ||
+         node->opcode() == IrOpcode::kInt32LessThanOrEqual ||
+         node->opcode() == IrOpcode::kUint32LessThan ||
+         node->opcode() == IrOpcode::kUint32LessThanOrEqual);
+  Int32BinopMatcher m(node);
+  // (x >>> K) < (y >>> K) => x < y   if only zeros shifted out
+  if (m.left().op() == machine()->Word32SarShiftOutZeros() &&
+      m.right().op() == machine()->Word32SarShiftOutZeros()) {
+    Int32BinopMatcher mleft(m.left().node());
+    Int32BinopMatcher mright(m.right().node());
+    if (mleft.right().HasValue() && mright.right().Is(mleft.right().Value())) {
+      node->ReplaceInput(0, mleft.left().node());
+      node->ReplaceInput(1, mright.left().node());
+      return Changed(node);
+    }
+  }
+  return NoChange();
+}
+
+const Operator* MachineOperatorReducer::Map64To32Comparison(
+    const Operator* op, bool sign_extended) {
+  switch (op->opcode()) {
+    case IrOpcode::kInt64LessThan:
+      return sign_extended ? machine()->Int32LessThan()
+                           : machine()->Uint32LessThan();
+    case IrOpcode::kInt64LessThanOrEqual:
+      return sign_extended ? machine()->Int32LessThanOrEqual()
+                           : machine()->Uint32LessThanOrEqual();
+    case IrOpcode::kUint64LessThan:
+      return machine()->Uint32LessThan();
+    case IrOpcode::kUint64LessThanOrEqual:
+      return machine()->Uint32LessThanOrEqual();
+    default:
+      UNREACHABLE();
+  }
+}
+
+Reduction MachineOperatorReducer::ReduceWord64Comparisons(Node* node) {
+  DCHECK(node->opcode() == IrOpcode::kInt64LessThan ||
+         node->opcode() == IrOpcode::kInt64LessThanOrEqual ||
+         node->opcode() == IrOpcode::kUint64LessThan ||
+         node->opcode() == IrOpcode::kUint64LessThanOrEqual);
+  Int64BinopMatcher m(node);
+
+  bool sign_extended =
+      m.left().IsChangeInt32ToInt64() && m.right().IsChangeInt32ToInt64();
+  if (sign_extended || (m.left().IsChangeUint32ToUint64() &&
+                        m.right().IsChangeUint32ToUint64())) {
+    node->ReplaceInput(0, NodeProperties::GetValueInput(m.left().node(), 0));
+    node->ReplaceInput(1, NodeProperties::GetValueInput(m.right().node(), 0));
+    NodeProperties::ChangeOp(node,
+                             Map64To32Comparison(node->op(), sign_extended));
+    return Changed(node).FollowedBy(Reduce(node));
+  }
+
+  // (x >>> K) < (y >>> K) => x < y   if only zeros shifted out
+  // This is useful for Smi untagging, which results in such a shift.
+  if (m.left().op() == machine()->Word64SarShiftOutZeros() &&
+      m.right().op() == machine()->Word64SarShiftOutZeros()) {
+    Int64BinopMatcher mleft(m.left().node());
+    Int64BinopMatcher mright(m.right().node());
+    if (mleft.right().HasValue() && mright.right().Is(mleft.right().Value())) {
+      node->ReplaceInput(0, mleft.left().node());
+      node->ReplaceInput(1, mright.left().node());
+      return Changed(node);
+    }
+  }
+
+  return NoChange();
+}
+
 Reduction MachineOperatorReducer::ReduceWord32Shifts(Node* node) {
   DCHECK((node->opcode() == IrOpcode::kWord32Shl) ||
          (node->opcode() == IrOpcode::kWord32Shr) ||
@@ -1275,14 +1354,42 @@ Reduction MachineOperatorReducer::ReduceWord32Shl(Node* node) {
         base::ShlWithWraparound(m.left().Value(), m.right().Value()));
   }
   if (m.right().IsInRange(1, 31)) {
-    // (x >>> K) << K => x & ~(2^K - 1)
-    // (x >> K) << K => x & ~(2^K - 1)
     if (m.left().IsWord32Sar() || m.left().IsWord32Shr()) {
       Int32BinopMatcher mleft(m.left().node());
+
+      // If x >> K only shifted out zeros:
+      // (x >> K) << L => x           if K == L
+      // (x >> K) << L => x >> (K-L) if K > L
+      // (x >> K) << L => x << (L-K)  if K < L
+      // Since this is used for Smi untagging, we currently only need it for
+      // signed shifts.
+      if (mleft.op() == machine()->Word32SarShiftOutZeros() &&
+          mleft.right().IsInRange(1, 31)) {
+        Node* x = mleft.left().node();
+        int k = mleft.right().Value();
+        int l = m.right().Value();
+        if (k == l) {
+          return Replace(x);
+        } else if (k > l) {
+          node->ReplaceInput(0, x);
+          node->ReplaceInput(1, Uint32Constant(k - l));
+          NodeProperties::ChangeOp(node, machine()->Word32Sar());
+          return Changed(node).FollowedBy(ReduceWord32Sar(node));
+        } else {
+          DCHECK(k < l);
+          node->ReplaceInput(0, x);
+          node->ReplaceInput(1, Uint32Constant(l - k));
+          return Changed(node);
+        }
+      }
+
+      // (x >>> K) << K => x & ~(2^K - 1)
+      // (x >> K) << K => x & ~(2^K - 1)
       if (mleft.right().Is(m.right().Value())) {
         node->ReplaceInput(0, mleft.left().node());
         node->ReplaceInput(1,
-                           Uint32Constant(~((1U << m.right().Value()) - 1U)));
+                           Uint32Constant(std::numeric_limits<uint32_t>::max()
+                                          << m.right().Value()));
         NodeProperties::ChangeOp(node, machine()->Word32And());
         return Changed(node).FollowedBy(ReduceWord32And(node));
       }
@@ -1299,6 +1406,46 @@ Reduction MachineOperatorReducer::ReduceWord64Shl(Node* node) {
     return ReplaceInt64(
         base::ShlWithWraparound(m.left().Value(), m.right().Value()));
   }
+  if (m.right().IsInRange(1, 63) &&
+      (m.left().IsWord64Sar() || m.left().IsWord64Shr())) {
+    Int64BinopMatcher mleft(m.left().node());
+
+    // If x >> K only shifted out zeros:
+    // (x >> K) << L => x           if K == L
+    // (x >> K) << L => x >> (K-L) if K > L
+    // (x >> K) << L => x << (L-K)  if K < L
+    // Since this is used for Smi untagging, we currently only need it for
+    // signed shifts.
+    if (mleft.op() == machine()->Word64SarShiftOutZeros() &&
+        mleft.right().IsInRange(1, 63)) {
+      Node* x = mleft.left().node();
+      int64_t k = mleft.right().Value();
+      int64_t l = m.right().Value();
+      if (k == l) {
+        return Replace(x);
+      } else if (k > l) {
+        node->ReplaceInput(0, x);
+        node->ReplaceInput(1, Uint64Constant(k - l));
+        NodeProperties::ChangeOp(node, machine()->Word64Sar());
+        return Changed(node).FollowedBy(ReduceWord64Sar(node));
+      } else {
+        DCHECK(k < l);
+        node->ReplaceInput(0, x);
+        node->ReplaceInput(1, Uint64Constant(l - k));
+        return Changed(node);
+      }
+    }
+
+    // (x >>> K) << K => x & ~(2^K - 1)
+    // (x >> K) << K => x & ~(2^K - 1)
+    if (mleft.right().Is(m.right().Value())) {
+      node->ReplaceInput(0, mleft.left().node());
+      node->ReplaceInput(1, Uint64Constant(std::numeric_limits<uint64_t>::max()
+                                           << m.right().Value()));
+      NodeProperties::ChangeOp(node, machine()->Word64And());
+      return Changed(node).FollowedBy(ReduceWord64And(node));
+    }
+  }
   return NoChange();
 }
 

src/compiler/machine-operator-reducer.h

@@ -92,6 +92,9 @@ class V8_EXPORT_PRIVATE MachineOperatorReducer final
   Reduction ReduceUint32Mod(Node* node);
   Reduction ReduceStore(Node* node);
   Reduction ReduceProjection(size_t index, Node* node);
+  const Operator* Map64To32Comparison(const Operator* op, bool sign_extended);
+  Reduction ReduceWord32Comparisons(Node* node);
+  Reduction ReduceWord64Comparisons(Node* node);
   Reduction ReduceWord32Shifts(Node* node);
   Reduction ReduceWord32Shl(Node* node);
   Reduction ReduceWord64Shl(Node* node);