Refactor FrameAndConstantPoolScope and ConstantPoolUnavailableScope to be architecture independent

src/macro-assembler.h

 // Copyright 2012 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.
 
 #ifndef V8_MACRO_ASSEMBLER_H_
 #define V8_MACRO_ASSEMBLER_H_
 
 
 // Helper types to make boolean flag easier to read at call-site.
 enum InvokeFlag {
@@ skipping 75 matching lines @@
 #include "src/x87/macro-assembler-x87.h"
 #else
 #error Unsupported target architecture.
 #endif
 
 namespace v8 {
 namespace internal {
 
 class FrameScope {
  public:
-  explicit FrameScope(MacroAssembler* masm, StackFrame::Type type)
+  explicit FrameScope(MacroAssembler* masm, StackFrame::Type type,
+                      bool enter_frame = true)

[rmcilroy, 2014/09/26 15:54:57]

I don't like this default argument. Rethinking this I think we should avoid
making FrameAndConstantPoolScope a subclass of FrameScope.  It doesn't buy us
much and there are subtle ordering constraints (like set_constant_pool_available
having to happen before masm->EnterFrame and after LeaveFrame) which are
difficult to ensure with this class inheritance. 

[baixo, 2014/10/02 09:18:21]

I was worried about code duplication, since this class is very similar to
FrameScope. But yes, the subclass approach will only work if the ordering
changes are not an issue. I reverted back to the original
FrameAndConstantPoolScope class.

       : masm_(masm), type_(type), old_has_frame_(masm->has_frame()) {
     masm->set_has_frame(true);
-    if (type != StackFrame::MANUAL && type_ != StackFrame::NONE) {
+    if (enter_frame && type != StackFrame::MANUAL &&
+        type_ != StackFrame::NONE) {
       masm->EnterFrame(type);
     }
   }
 
   ~FrameScope() {
     if (type_ != StackFrame::MANUAL && type_ != StackFrame::NONE) {
       masm_->LeaveFrame(type_);
     }
     masm_->set_has_frame(old_has_frame_);
   }
 
   // Normally we generate the leave-frame code when this object goes
   // out of scope.  Sometimes we may need to generate the code somewhere else
   // in addition.  Calling this will achieve that, but the object stays in
   // scope, the MacroAssembler is still marked as being in a frame scope, and
   // the code will be generated again when it goes out of scope.
   void GenerateLeaveFrame() {
     DCHECK(type_ != StackFrame::MANUAL && type_ != StackFrame::NONE);
     masm_->LeaveFrame(type_);
   }
 
- private:
+ protected:
   MacroAssembler* masm_;
   StackFrame::Type type_;
   bool old_has_frame_;
 };
 
+class FrameAndConstantPoolScope : public FrameScope {
+ public:
+  FrameAndConstantPoolScope(MacroAssembler* masm, StackFrame::Type type)
+      : FrameScope(masm, type, false),
+        old_constant_pool_available_(masm->is_constant_pool_available()) {
+    // We only want to enable constant pool access for non-manual frame scopes
+    // to ensure the constant pool pointer is valid throughout the scope.
+    DCHECK(type_ != StackFrame::MANUAL && type_ != StackFrame::NONE);
+    masm->set_constant_pool_available(true);
+    masm->EnterFrame(type, !old_constant_pool_available_);
+  }
+
+  ~FrameAndConstantPoolScope() {
+    masm_->set_constant_pool_available(old_constant_pool_available_);
+  }
+
+ private:
+  bool old_constant_pool_available_;
+
+  DISALLOW_IMPLICIT_CONSTRUCTORS(FrameAndConstantPoolScope);
+};
+
 
 class AllowExternalCallThatCantCauseGC: public FrameScope {
  public:
   explicit AllowExternalCallThatCantCauseGC(MacroAssembler* masm)
       : FrameScope(masm, StackFrame::NONE) { }
 };
 
 
 class NoCurrentFrameScope {
  public:
@@ skipping 59 matching lines @@
           isolate);
     }
     return ExternalReference::new_space_allocation_limit_address(isolate);
   }
 };
 
 
 } }  // namespace v8::internal
 
 #endif  // V8_MACRO_ASSEMBLER_H_