Author: [EMAIL PROTECTED]
Date: Fri Oct 31 04:55:06 2008
New Revision: 671

Modified:
    branches/bleeding_edge/src/codegen-arm.cc
    branches/bleeding_edge/src/codegen-ia32.cc

Log:
Simplify the way we materialize boolean values that are not yet pushed
on the stack frame.
Review URL: http://codereview.chromium.org/8764

Modified: branches/bleeding_edge/src/codegen-arm.cc
==============================================================================
--- branches/bleeding_edge/src/codegen-arm.cc   (original)
+++ branches/bleeding_edge/src/codegen-arm.cc   Fri Oct 31 04:55:06 2008
@@ -359,47 +359,41 @@
    Label false_target;
    LoadCondition(x, typeof_state, &true_target, &false_target, false);

+  // Materialize boolean values on the stack frame if necessary.
    if (has_cc()) {
-    // convert cc_reg_ into a bool
-    Label loaded, materialize_true;
-    __ b(cc_reg_, &materialize_true);
+    // The value we want is in the cc register.  Since complicated
+    // conditions can require more than one test, there may also be  
branches
+    // to true_target and false_target.
+    Label loaded;
+    __ b(cc_reg_, &true_target);
+    __ bind(&false_target);
      __ mov(r0, Operand(Factory::false_value()));
      __ push(r0);
      __ b(&loaded);
-    __ bind(&materialize_true);
+    __ bind(&true_target);
      __ mov(r0, Operand(Factory::true_value()));
      __ push(r0);
      __ bind(&loaded);
      cc_reg_ = al;
-  }
+  } else {
+    // Optimization of boolean-valued expressions whose value is statically
+    // known may have generated an unconditional jump to either of the
+    // targets (but not both) and failed to leave a value in either the cc
+    // register or on top of the frame.
+    ASSERT(!(true_target.is_linked() && false_target.is_linked()));

-  if (true_target.is_linked() || false_target.is_linked()) {
-    // we have at least one condition value
-    // that has been "translated" into a branch,
-    // thus it needs to be loaded explicitly again
-    Label loaded;
-    __ b(&loaded);  // don't lose current TOS
-    bool both = true_target.is_linked() && false_target.is_linked();
-    // reincarnate "true", if necessary
      if (true_target.is_linked()) {
        __ bind(&true_target);
        __ mov(r0, Operand(Factory::true_value()));
        __ push(r0);
      }
-    // if both "true" and "false" need to be reincarnated,
-    // jump across code for "false"
-    if (both)
-      __ b(&loaded);
-    // reincarnate "false", if necessary
+
      if (false_target.is_linked()) {
        __ bind(&false_target);
        __ mov(r0, Operand(Factory::false_value()));
        __ push(r0);
      }
-    // everything is loaded at this point
-    __ bind(&loaded);
    }
-  ASSERT(!has_cc());
  }



Modified: branches/bleeding_edge/src/codegen-ia32.cc
==============================================================================
--- branches/bleeding_edge/src/codegen-ia32.cc  (original)
+++ branches/bleeding_edge/src/codegen-ia32.cc  Fri Oct 31 04:55:06 2008
@@ -474,44 +474,37 @@
    Label false_target;
    LoadCondition(x, typeof_state, &true_target, &false_target, false);

+  // Materialize boolean values on the stack frame if necessary.
    if (has_cc()) {
-    // convert cc_reg_ into a bool
-
-    Label loaded, materialize_true;
-    __ j(cc_reg_, &materialize_true);
+    // The value we want is in the cc register.  Since complicated
+    // conditions can require more than one test, there may also be  
branches
+    // to true_target and false_target.
+    Label loaded;
+    __ j(cc_reg_, &true_target);
+    __ bind(&false_target);
      frame_->Push(Immediate(Factory::false_value()));
      __ jmp(&loaded);
-    __ bind(&materialize_true);
+    __ bind(&true_target);
      frame_->Push(Immediate(Factory::true_value()));
      __ bind(&loaded);
      cc_reg_ = no_condition;
-  }
+  } else {
+    // Optimization of boolean-valued expressions whose value is statically
+    // known may have generated an unconditional jump to either of the
+    // targets (but not both) and failed to leave a value in either the cc
+    // register or on top of the frame.
+    ASSERT(!(true_target.is_linked() && false_target.is_linked()));

-  if (true_target.is_linked() || false_target.is_linked()) {
-    // we have at least one condition value
-    // that has been "translated" into a branch,
-    // thus it needs to be loaded explicitly again
-    Label loaded;
-    __ jmp(&loaded);  // don't lose current TOS
-    bool both = true_target.is_linked() && false_target.is_linked();
-    // reincarnate "true", if necessary
      if (true_target.is_linked()) {
        __ bind(&true_target);
        frame_->Push(Immediate(Factory::true_value()));
      }
-    // if both "true" and "false" need to be reincarnated,
-    // jump across code for "false"
-    if (both)
-      __ jmp(&loaded);
-    // reincarnate "false", if necessary
+
      if (false_target.is_linked()) {
        __ bind(&false_target);
        frame_->Push(Immediate(Factory::false_value()));
      }
-    // everything is loaded at this point
-    __ bind(&loaded);
    }
-  ASSERT(!has_cc());
  }



--~--~---------~--~----~------------~-------~--~----~
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
-~----------~----~----~----~------~----~------~--~---

Reply via email to