Made changes to moved code in MergeMoveMemoryToRegister, seemed to be off by one. Passes V8 and Mozilla tests, but it passed them before, too.
http://codereview.chromium.org/14158/diff/1/2 File src/virtual-frame-ia32.cc (right): http://codereview.chromium.org/14158/diff/1/2#newcode584 Line 584: Immediate((stack_pointer_ - i) * kPointerSize)); On 2008/12/17 00:46:18, iposva wrote: > At this point stack_pointer_ is (i + 1). Thus stack_pointer_ - i == (i + 1) - i > == 1 is the value being added to esp. I do not think this is the intention. Are > you sure this code was tested? I can imagine that it is pretty hard to generate > such a case which makes it really mandatory to ensure that this code works. > Otherwise you can run for years with such a bug in the code generator without > ever finding out about it. > > More comments about your intentions would probably let you come to the same > conclusion while writing the code. Alternatively you could be less frugal with > your local variables and make your intentions explicit in the code (aka comments > get stale, code at least has to compile): > > int new_stack_pointer = i + 1; > ASSERT((stack_pointer_ - new_stack_pointer) > 0); // I know this assertion is a > duplicate of the above if check but it documents the precondition for the add > here and it ensures that the if check does not get changed. > __ add(Operand(esp), Immediate((stack_pointer_ - new_stack_pointer) * > kPointerSize); > stack_pointer_ = new_stack_pointer; > > Amount added was also wrong. More changes made. Seemed to have a pervasive off-by-one error. Needs review. http://codereview.chromium.org/14158/diff/1/2#newcode601 Line 601: } On 2008/12/17 00:46:18, iposva wrote: > Unless I am missing something about merging frames, both stack_pointer_ values > should be equal at this point. Please assert that, alternatively this and > everything else should be asserted in MergeTo(). Everything is checked in MergeTo(), and the checking code has been factored in a separate change. http://codereview.chromium.org/14158 --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
