Support non-internalized string key lookups in Hydrogen KeyedLoadIC

src/hydrogen.cc

 // Copyright 2013 the V8 project authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "src/hydrogen.h"
 
 #include <algorithm>
 
 #include "src/v8.h"
 
@@ skipping 1495 matching lines @@
           Push(BuildDecodeField<String::ArrayIndexValueBits>(hash));
         }
         string_index_if.Else();
         {
           // Key is a non-index String, check for uniqueness/internalization. If
           // it's not, deopt.
           HValue* not_internalized_bit = AddUncasted<HBitwise>(
               Token::BIT_AND,
               instance_type,
               Add<HConstant>(static_cast<int>(kIsNotInternalizedMask)));
-          DeoptimizeIf<HCompareNumericAndBranch>(
-              not_internalized_bit,
-              graph()->GetConstant0(),
-              Token::NE,
-              "BuildKeyedIndexCheck: string isn't internalized");
-          // Key guaranteed to be a unqiue string
+
+          IfBuilder not_internalized(this);
+          not_internalized.If<HCompareNumericAndBranch>(not_internalized_bit,
+                                                        graph()->GetConstant0(),
+                                                        Token::EQ);
+          not_internalized.Then();

[Jakob Kummerow, 2014/06/27 17:37:37]

Now you've changed both the name and the meaning. (Or am I confused?)
Either the IfBuilder should be called "internalized", then the condition should
be "not_internalized_bit == 0", and the Then() part should push the key while
the Else() part calls Runtime_InternalizeString.
Or it should be called not_internalized, with the condition being
"not_internalized != 0", the Then() branch calling Internalize and the Else()
branch pushing the original key (as the not_internalized bit is not unequal to
0... quadruple negation ftw!).

It would probably make sense to add a
RUNTIME_ASSERT(!StringShape(*string)->IsInternalized()) to
Runtime_InternalizeString...

[danno, 2014/06/27 17:43:28]

This is intentional. I also switched from Token::NE to Token::EQ. This generates
better code, since the "then" case which doesn't have to do any work jumps over
the "else" case that does. But your right, the name is now flipped. I'll fix
that

As far as the runtime assert, it's not needed, since Factory does the right
thing with already internalized strings.

           Push(key);
+
+          not_internalized.Else();
+          Add<HPushArguments>(key);
+          HValue* intern_key = Add<HCallRuntime>(
+              isolate()->factory()->empty_string(),
+              Runtime::FunctionForId(Runtime::kInternalizeString), 1);
+          Push(intern_key);
+
+          not_internalized.End();
+          // Key guaranteed to be a unique string
         }
         string_index_if.JoinContinuation(join_continuation);
       }
       not_symbol_if.Else();
       {
         Push(key);  // Key is symbol
       }
       not_symbol_if.JoinContinuation(join_continuation);
     }
     not_string_or_name_if.JoinContinuation(join_continuation);
@@ skipping 10819 matching lines @@
   if (ShouldProduceTraceOutput()) {
     isolate()->GetHTracer()->TraceHydrogen(name(), graph_);
   }
 
 #ifdef DEBUG
   graph_->Verify(false);  // No full verify.
 #endif
 }
 
 } }  // namespace v8::internal