Disable guessing 'other' cid; this prevents an issue in range analysis. It is not clear if guessing the 'other' cid optimization is the right thing to do as it may bring the flow graph in an unexpected state

BUG=
R=johnmccutchan@google.com

Review URL: https://codereview.chromium.org//1203773002.
This commit is contained in:
Srdjan Mitrovic 2015-06-23 13:23:02 -07:00
parent 1ce49788ea
commit 28594e455d
5 changed files with 10 additions and 12 deletions

View file

@ -71,7 +71,6 @@ DECLARE_FLAG(bool, trace_patching);
bool Compiler::always_optimize_ = false;
bool Compiler::guess_other_cid_ = true;
// TODO(zerny): Factor out unoptimizing/optimizing pipelines and remove

View file

@ -90,13 +90,8 @@ class Compiler : public AllStatic {
static bool always_optimize() { return always_optimize_; }
static void set_always_optimize(bool value) { always_optimize_ = value; }
// Default: true.
static bool guess_other_cid() { return guess_other_cid_; }
static void set_guess_other_cid(bool value) { guess_other_cid_ = value; }
private:
static bool always_optimize_;
static bool guess_other_cid_;
};
} // namespace dart

View file

@ -51,6 +51,7 @@ DECLARE_FLAG(charp, deoptimize_filter);
DECLARE_FLAG(bool, disassemble);
DECLARE_FLAG(bool, disassemble_optimized);
DECLARE_FLAG(bool, emit_edge_counters);
DECLARE_FLAG(bool, guess_other_cid);
DECLARE_FLAG(bool, ic_range_profiling);
DECLARE_FLAG(bool, intrinsify);
DECLARE_FLAG(bool, load_deferred_eagerly);
@ -82,8 +83,8 @@ static void NooptModeHandler(bool value) {
FLAG_deoptimize_alot = false; // Used in some tests.
FLAG_deoptimize_every = 0; // Used in some tests.
FLAG_collect_code = false;
FLAG_guess_other_cid = false;
Compiler::set_always_optimize(true);
Compiler::set_guess_other_cid(false);
// TODO(srdjan): Enable CHA deoptimization when eager class finalization is
// implemented, either with precompilation or as a special pass.
FLAG_use_cha_deopt = false;

View file

@ -27,6 +27,11 @@ namespace dart {
DEFINE_FLAG(int, getter_setter_ratio, 13,
"Ratio of getter/setter usage used for double field unboxing heuristics");
// Setting 'guess_other_cid' to true causes issue 23693 crash.
// TODO(srdjan): Evaluate if that optimization is wrong.
DEFINE_FLAG(bool, guess_other_cid, false,
"Artificially create type feedback for arithmetic etc. operations"
" by guessing the other unknown argument cid");
DEFINE_FLAG(bool, load_cse, true, "Use redundant load elimination.");
DEFINE_FLAG(bool, dead_store_elimination, true, "Eliminate dead stores");
DEFINE_FLAG(int, max_polymorphic_checks, 4,
@ -196,7 +201,9 @@ bool FlowGraphOptimizer::TryCreateICData(InstanceCallInstr* call) {
Token::IsBinaryOperator(op_kind)) {
// Guess cid: if one of the inputs is a number assume that the other
// is a number of same type.
if (Compiler::guess_other_cid()) {
// Issue 23693. It is potentially wrong to assign types here that may
// conflict with other graph analysis.
if (FLAG_guess_other_cid) {
const intptr_t cid_0 = class_ids[0];
const intptr_t cid_1 = class_ids[1];
if ((cid_0 == kDynamicCid) && (IsNumberCid(cid_1))) {

View file

@ -205,10 +205,6 @@ regexp/global_test: Skip # Timeout. Issue 21709 and 21708
[ $mode == debug ]
regexp/pcre_test: Pass, Slow # Timeout. Issue 22008
[ $mode == debug && $runtime == vm]
big_integer_arith_vm_test/gcd: Pass, Crash # Issue 23693
big_integer_arith_vm_test/modInv: Pass, Crash # Issue 23693
[ $runtime == vm && $arch == simarmv5te ]
int_parse_radix_test/*: Pass, Slow
big_integer_parsed_mul_div_vm_test: Pass, Slow