Base iDef update code.

src/hydrogen-instructions.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 561 matching lines @@
     kIsArguments,
     kTruncatingToInt32,
     kIsDead,
     // Instructions that are allowed to produce full range unsigned integer
     // values are marked with kUint32 flag. If arithmetic shift or a load from
     // EXTERNAL_UNSIGNED_INT_ELEMENTS array is not marked with this flag
     // it will deoptimize if result does not fit into signed integer range.
     // HGraph::ComputeSafeUint32Operations is responsible for setting this
     // flag.
     kUint32,
-    kLastFlag = kUint32
+    // This flag is set to true after the SetupInformativeDefinitions() pass
+    // has processed this instruction.
+    kIDefsProcessingDone,
+    kLastFlag = kIDefsProcessingDone
   };
 
   STATIC_ASSERT(kLastFlag < kBitsPerInt);
 
   static const int kChangesToDependsFlagsLeftShift = 1;
 
   static GVNFlag ChangesFlagFromInt(int x) {
     return static_cast<GVNFlag>(x * 2);
   }
   static GVNFlag DependsOnFlagFromInt(int x) {
@@ skipping 87 matching lines @@
   // One of the iDef operands is special because it is the value that is
   // "transferred" to the output, we call it the "redefined operand".
   // If an HValue is an iDef it must override RedefinedOperandIndex() so that
   // it does not return kNoRedefinedOperand;
   static const int kNoRedefinedOperand = -1;
   virtual int RedefinedOperandIndex() { return kNoRedefinedOperand; }
   bool IsInformativeDefinition() {
     return RedefinedOperandIndex() != kNoRedefinedOperand;
   }
   HValue* RedefinedOperand() {
-    ASSERT(IsInformativeDefinition());
-    return OperandAt(RedefinedOperandIndex());
+    return IsInformativeDefinition() ? OperandAt(RedefinedOperandIndex())
+                                     : NULL;
   }
 
   // This method must always return the original HValue SSA definition
   // (regardless of any iDef of this value).
   HValue* ActualValue() {
     return IsInformativeDefinition() ? RedefinedOperand()->ActualValue()
                                      : this;
   }
 
+  virtual void AddInformativeDefinitions() {}
+
+  void UpdateRedefinedUsesWhileSettingUpInformativeDefinitions() {
+    UpdateRedefinedUsesInner<CheckDominanceUsingProcessedFlag>();
+  }
+  void UpdateRedefinedUses() {
+    UpdateRedefinedUsesInner<CheckDominanceFollowingDefinitionList>();
+  }
+
   bool IsDefinedAfter(HBasicBlock* other) const;
 
   // Operands.
   virtual int OperandCount() = 0;
   virtual HValue* OperandAt(int index) const = 0;
   void SetOperandAt(int index, HValue* value);
 
   void DeleteAndReplaceWith(HValue* other);
   void ReplaceAllUsesWith(HValue* other);
   bool HasNoUses() const { return use_list_ == NULL; }
@@ skipping 138 matching lines @@
   void clear_block() {
     ASSERT(block_ != NULL);
     block_ = NULL;
   }
 
   void set_representation(Representation r) {
     ASSERT(representation_.IsNone() && !r.IsNone());
     representation_ = r;
   }
 
+  // Check if "dominator" properly dominates "dominated"
+  // (used in UpdateRedefinedUsesInner).
+  static bool CheckDominanceFollowingDefinitionList(

[Jakob Kummerow, 2013/01/31 14:06:43]

I'm not too happy with this method's name. "Check" sounds like some assertion.
Also, it would be nice to model this after HBasicBlock::Dominates(). Lastly, I'm
not sure what "FollowingDefinitionList" tries to tell me.
So how about "DominatesByBlockWalking"? And "DominatesByProcessedFlag" below? Or
just "DominatesSlow"/"DominatesFast"?

Ideally, I'd make it a non-static method too (so you can call it as
"dominator->DominatesSlow(other)", but that doesn't fit with passing method
pointers (see below). For templatized use, it would work if the template
parameter was a bool or enum used in an if/else (which the compiler would
optimize away).

[Massi, 2013/01/31 17:16:19]

Renamed the methods but, after the offline chat, kept the template based
implementation.

+      HValue* dominator, HValue* dominated);

[Jakob Kummerow, 2013/01/31 14:06:43]

nit: for method/function declarations (contrary to calls), put each argument
onto its own line unless everything (including the method name) fits onto one
line. In this case:

  static bool CheckDominanceFollowingDefinitionList(HValue* dominator,
                                                    HValue* dominated);

[Massi, 2013/01/31 17:16:19]

Done.

+
+  // When we are setting up informaative definitions we use a flag to mark

[Jakob Kummerow, 2013/01/31 14:06:43]

nit: s/informaative/informative/

[Massi, 2013/01/31 17:16:19]

Done.

+  // processed instructions, and by checking the flag we know if an
+  // instruction comes before or after the "current" one.
+  static bool CheckDominanceUsingProcessedFlag(
+      HValue* dominator, HValue* dominated);

[Jakob Kummerow, 2013/01/31 14:06:43]

same here

[Massi, 2013/01/31 17:16:19]

Done.

+
+  // If we are redefining an operand, update all its dominated uses (the
+  // function that checks if a use is dominated is the template argument).
+  template<bool (*CheckDominance)(HValue*, HValue*)>

[Jakob Kummerow, 2013/01/31 14:06:43]

Please use a typedef for the function signature definition. (Put it right before
the first declaration of a function of this type; even though syntactically they
don't use it, they do semantically "implement the interface" defined by the
typedef.)

Is there any particular reason you templatized this method, as opposed to just
passing in a function pointer?

[Massi, 2013/01/31 17:16:19]

Done.

+  void UpdateRedefinedUsesInner() {
+    HValue* input = RedefinedOperand();
+    if (input != NULL) {
+      HUseIterator uses = input->uses();
+      while (!uses.Done()) {

[Jakob Kummerow, 2013/01/31 14:06:43]

I'm used to seeing for-loops for iterating over uses. Let's be consistent with
that pattern:

for (HUseIterator uses = input->uses(); !uses.Done(); uses.Advance()) { ... }

[Massi, 2013/01/31 17:16:19]

Done.

+        HValue* use = uses.value();
+        int index = uses.index();
+        uses.Advance();
+        // We need to update all dominated uses.
+        if (CheckDominance(this, use)) {
+          use->SetOperandAt(index, this);
+        }
+      }
+    }
+  }
+
   static GVNFlagSet AllDependsOnFlagSet() {
     GVNFlagSet result;
     // Create changes mask.
 #define ADD_FLAG(type) result.Add(kDependsOn##type);
   GVN_TRACKED_FLAG_LIST(ADD_FLAG)
   GVN_UNTRACKED_FLAG_LIST(ADD_FLAG)
 #undef ADD_FLAG
     return result;
   }
 
@@ skipping 4698 matching lines @@
   virtual bool IsDeletable() const { return true; }
 };
 
 
 #undef DECLARE_INSTRUCTION
 #undef DECLARE_CONCRETE_INSTRUCTION
 
 } }  // namespace v8::internal
 
 #endif  // V8_HYDROGEN_INSTRUCTIONS_H_