Encapsulating Type information in the CompareICStub

src/code-stubs.h

 // Copyright 2012 the V8 project authors. All rights reserved.
 // Redistribution and use in source and binary forms, with or without
 // modification, are permitted provided that the following conditions are
 // met:
 //
 //     * Redistributions of source code must retain the above copyright
 //       notice, this list of conditions and the following disclaimer.
 //     * Redistributions in binary form must reproduce the above
 //       copyright notice, this list of conditions and the following
 //       disclaimer in the documentation and/or other materials provided
@@ skipping 1029 matching lines @@
   Token::Value op_;
   CompareIC::State left_;
   CompareIC::State right_;
   CompareIC::State state_;
   Handle<Map> known_map_;
 };
 
 
 class CompareNilICStub : public HydrogenCodeStub  {
  public:
-  enum Types {
-    kCompareAgainstNull = 1 << 0,
-    kCompareAgainstUndefined = 1 << 1,
-    kCompareAgainstMonomorphicMap = 1 << 2,
-    kCompareAgainstUndetectable = 1 << 3,
-    kFullCompare = kCompareAgainstNull | kCompareAgainstUndefined |
-        kCompareAgainstUndetectable
+  enum Type {
+    UNDEFINED,
+    NULL_TYPE,
+    MONOMORPHIC_MAP,
+    UNDETECTABLE,
+    NUMBER_OF_TYPES
   };
 
+  class Types : public EnumSet<Type, byte> {
+   public:
+    Types() : EnumSet<Type, byte>(0) { }
+    explicit Types(byte bits) : EnumSet<Type, byte>(bits) { }
+
+    static const Types FullCompare() {

[Sven Panne, 2013/05/14 09:18:42]

No need for "const" here, we have an implicit copy, anyway.

+      Types set;
+      set.Add(UNDEFINED);
+      set.Add(NULL_TYPE);
+      set.Add(UNDETECTABLE);
+      return set;
+    }
+
+    bool IsFullCompare() const {

[Sven Panne, 2013/05/14 09:18:42]

No need for this function, we have FullCompare() and operator==().

+      return this->ToByte() == FullCompare().ToByte();
+    }
+
+    void SetTo(Type type);

[Sven Panne, 2013/05/14 09:18:42]

We don't need this, pretending that Types is immutable and just assigning is
clearer. If you like, you could add a constructor/factory method for a singleton
to polish the call sites a bit, but that's up to you.

+    byte ToByte() const { return ToIntegral(); }

[Sven Panne, 2013/05/14 09:18:42]

Remove this, just use ToIntegral().

+
+    void Print(StringStream* stream) const;
+  };
+
+  // At most 6 different types can be distinguished, because the Code object
+  // only has room for a single byte to hold a set and there are two more
+  // boolean flags we need to store. :-P
+  STATIC_ASSERT(NUMBER_OF_TYPES <= 6);
+
   CompareNilICStub(EqualityKind kind, NilValue nil, Types types)
-      : HydrogenCodeStub(CODE_STUB_IS_NOT_MISS), bit_field_(0) {
-    bit_field_ = EqualityKindField::encode(kind) |
-        NilValueField::encode(nil) |
-        TypesField::encode(types);
+      : HydrogenCodeStub(CODE_STUB_IS_NOT_MISS), types_(types) {
+    equality_kind_ = kind;
+    nil_value_ = nil;
   }
 
-  virtual InlineCacheState GetICState() {
-    Types types = GetTypes();
-    if (types == kFullCompare) {
-      return MEGAMORPHIC;
-    } else if ((types & kCompareAgainstMonomorphicMap) != 0) {
-      return MONOMORPHIC;
-    } else {
-      return PREMONOMORPHIC;
-    }
+  explicit CompareNilICStub(Code::ExtraICState ic_state)
+      : HydrogenCodeStub(CODE_STUB_IS_NOT_MISS) {
+    equality_kind_ = EqualityKindField::decode(ic_state);
+    nil_value_ = NilValueField::decode(ic_state);
+    types_ = Types(ExtractTypesFromExtraICState(ic_state));
   }
 
-  virtual Code::Kind GetCodeKind() const { return Code::COMPARE_NIL_IC; }
-
-  Handle<Code> GenerateCode();
-
   static Handle<Code> GetUninitialized(Isolate* isolate,
                                        EqualityKind kind,
                                        NilValue nil) {
-    return CompareNilICStub(kind, nil).GetCode(isolate);
+    return CompareNilICStub(kind, nil, CODE_STUB_IS_MISS).GetCode(isolate);
   }
 
   virtual void InitializeInterfaceDescriptor(
       Isolate* isolate,
       CodeStubInterfaceDescriptor* descriptor);
 
   static void InitializeForIsolate(Isolate* isolate) {
-    CompareNilICStub compare_stub(kStrictEquality, kNullValue);
+    CompareNilICStub compare_stub(kStrictEquality, kNullValue,
+                                  CODE_STUB_IS_MISS);
     compare_stub.InitializeInterfaceDescriptor(
         isolate,
         isolate->code_stub_interface_descriptor(CodeStub::CompareNilIC));
   }
 
-  virtual Code::ExtraICState GetExtraICState() {
-    return bit_field_;
+  virtual InlineCacheState GetICState() {
+    if (types_.IsFullCompare()) {
+      return MEGAMORPHIC;
+    } else if (types_.Contains(MONOMORPHIC_MAP)) {
+      return MONOMORPHIC;
+    } else {
+      return PREMONOMORPHIC;
+    }
   }
 
-  EqualityKind GetKind() { return EqualityKindField::decode(bit_field_); }
-  NilValue GetNilValue() { return NilValueField::decode(bit_field_); }
-  Types GetTypes() { return TypesField::decode(bit_field_); }
+  virtual Code::Kind GetCodeKind() const { return Code::COMPARE_NIL_IC; }
 
-  static Types TypesFromExtraICState(
+  Handle<Code> GenerateCode();
+
+  // extra ic state = nil_value | equality_kind | type_n-1 | ... | type_0
+  virtual Code::ExtraICState GetExtraICState() {
+    return NilValueField::encode(nil_value_)         |
+           EqualityKindField::encode(equality_kind_) |
+           types_.ToByte();
+  }
+  static byte ExtractTypesFromExtraICState(
       Code::ExtraICState state) {
-    return TypesField::decode(state);
-  }
-  static EqualityKind EqualityKindFromExtraICState(
-      Code::ExtraICState state) {
-    return EqualityKindField::decode(state);
-  }
-  static NilValue NilValueFromExtraICState(Code::ExtraICState state) {
-    return NilValueField::decode(state);
+    return state & ((1<<NUMBER_OF_TYPES)-1);
   }
 
-  static Types GetPatchedICFlags(Code::ExtraICState extra_ic_state,
-                                 Handle<Object> object,
-                                 bool* already_monomorphic);
+  void Record(Handle<Object> object);
+
+  bool IsMonomorphic() const { return types_.Contains(MONOMORPHIC_MAP); }
+  EqualityKind GetKind() const { return equality_kind_; }
+  NilValue GetNilValue() const { return nil_value_; }
+  const Types GetTypes() const { return types_; }

[Sven Panne, 2013/05/14 09:18:42]

The first 'const' doesn't really make sense: It doesn't force the callee to
assign the result to e.g. a 'const Types foo'. Furthermore, we implicitly copy
types_, anyway.

+  void ClearTypes() { types_.RemoveAll(); }
+  void SetKind(EqualityKind kind) { equality_kind_ = kind; }
+
+  virtual void PrintName(StringStream* stream);
 
  private:
   friend class CompareNilIC;
 
-  class EqualityKindField : public BitField<EqualityKind, 0, 1> {};
-  class NilValueField : public BitField<NilValue, 1, 1> {};
-  class TypesField : public BitField<Types, 3, 4> {};
-
-  CompareNilICStub(EqualityKind kind, NilValue nil)
-      : HydrogenCodeStub(CODE_STUB_IS_MISS), bit_field_(0) {
-    bit_field_ = EqualityKindField::encode(kind) |
-        NilValueField::encode(nil);
+  CompareNilICStub(EqualityKind kind, NilValue nil,
+                   InitializationState init_state)
+      : HydrogenCodeStub(init_state), types_(0) {
+    equality_kind_ = kind;
+    nil_value_ = nil;
   }
 
+  CompareNilICStub(Code::ExtraICState ic_state, InitializationState init_state)
+      : HydrogenCodeStub(init_state) {
+    equality_kind_ = EqualityKindField::decode(ic_state);
+    nil_value_ = NilValueField::decode(ic_state);
+    types_ = Types(ExtractTypesFromExtraICState(ic_state));
+  }
+
+  class EqualityKindField : public BitField<EqualityKind, NUMBER_OF_TYPES, 1> {
+  };
+  class NilValueField : public BitField<NilValue, NUMBER_OF_TYPES+1, 1> {};
+
   virtual CodeStub::Major MajorKey() { return CompareNilIC; }
-  virtual int NotMissMinorKey() { return bit_field_; }
+  virtual int NotMissMinorKey() { return GetExtraICState(); }
 
-  int bit_field_;
+  EqualityKind equality_kind_;
+  NilValue nil_value_;
+  Types types_;
 
   DISALLOW_COPY_AND_ASSIGN(CompareNilICStub);
 };
 
 
 class CEntryStub : public PlatformCodeStub {
  public:
   explicit CEntryStub(int result_size,
                       SaveFPRegsMode save_doubles = kDontSaveFPRegs)
       : result_size_(result_size), save_doubles_(save_doubles) { }
@@ skipping 639 matching lines @@
     STRING,
     SYMBOL,
     HEAP_NUMBER,
     NUMBER_OF_TYPES
   };
 
   // At most 8 different types can be distinguished, because the Code object
   // only has room for a single byte to hold a set of these types. :-P
   STATIC_ASSERT(NUMBER_OF_TYPES <= 8);
 
-  class Types {
+  class Types : public EnumSet<Type, byte> {
    public:
     Types() {}
-    explicit Types(byte bits) : set_(bits) {}
+    explicit Types(byte bits) : EnumSet<Type, byte>(bits) {}
 
-    bool IsEmpty() const { return set_.IsEmpty(); }
-    bool Contains(Type type) const { return set_.Contains(type); }
-    bool ContainsAnyOf(Types types) const {
-      return set_.ContainsAnyOf(types.set_);
-    }
-    void Add(Type type) { set_.Add(type); }
-    byte ToByte() const { return set_.ToIntegral(); }
+    byte ToByte() const { return ToIntegral(); }

[Sven Panne, 2013/05/14 09:18:42]

Just using ToIntegral() is easier here, too. (I know this has been there before
your change... :-)

[oliv, 2013/05/14 13:28:11]

This is still called from platform dependant code, which i anyway want to
completely remove in a later cl. so i'm not gonna remove that in this cl

     void Print(StringStream* stream) const;
     void TraceTransition(Types to) const;
     bool Record(Handle<Object> object);
     bool NeedsMap() const;
     bool CanBeUndetectable() const;
-
-   private:
-    EnumSet<Type, byte> set_;
   };
 
   static Types no_types() { return Types(); }
   static Types all_types() { return Types((1 << NUMBER_OF_TYPES) - 1); }
 
   explicit ToBooleanStub(Register tos, Types types = Types())
       : tos_(tos), types_(types) { }
 
   void Generate(MacroAssembler* masm);
   virtual Code::Kind GetCodeKind() const { return Code::TO_BOOLEAN_IC; }
@@ skipping 136 matching lines @@
 
   // The current function entry hook.
   static FunctionEntryHook entry_hook_;
 
   DISALLOW_COPY_AND_ASSIGN(ProfileEntryHookStub);
 };
 
 } }  // namespace v8::internal
 
 #endif  // V8_CODE_STUBS_H_