Fix reintroduction of global variables that have been deleted.

Fix reintroduction of global variables that have been deleted.

Deletion of global properties puts 'the hole' in the global property
cell and updates the property details in the property dictionary with
the information that the property has been deleted. When setting
global properties that have been deleted in generated code we just
store the new value in the global property cell. This does not update
the property details in the property dictionary. Therefore, it looks
like the property is not there eventhough it was just reintroduced.

Perform 'the hole' checks in generated code for global property stores
and bail out of ICs and optimized code if storing to a property cell
that contains 'the hole'.

Committed: http://code.google.com/p/v8/source/detail?r=6508

[Kevin Millikin (Chromium), 2011-01-27 05:27:54]

LGTM.

http://codereview.chromium.org/6306014/diff/1/src/x64/stub-cache-x64.cc
File src/x64/stub-cache-x64.cc (right):

http://codereview.chromium.org/6306014/diff/1/src/x64/stub-cache-x64.cc#newco...
src/x64/stub-cache-x64.cc:2447: __ cmpq(kScratchRegister,
I think theres a compare root function on x64 (and possibly ARM?)

[Mads Ager (chromium), 2011-01-27 07:13:34]

Thanks Kevin. I used CompareRoot on x64 (not there on ARM). Also, I changed
hydrogen and lithium so we only emit the extra check if the cell is deletable.
It is now treated in the same way as LoadGlobal. Could you have a quick look at
the hydrogen and lithium changes?

[Kevin Millikin (Chromium), 2011-01-27 07:56:07]

Still LGTM.  It would be great if we could come up with some easy tests for the
path in optimized code that needs the check.

http://codereview.chromium.org/6306014/diff/5001/src/arm/lithium-arm.cc
File src/arm/lithium-arm.cc (right):

http://codereview.chromium.org/6306014/diff/5001/src/arm/lithium-arm.cc#newco...
src/arm/lithium-arm.cc:1605: LOperand* temp = instr->check_hole_value() ?
TempRegister() : NULL;
OK, but since almost every line has to check the same flag, I'd split it into:

if (instr->check_hole_value()) {
  LOperand* temp = TempRegister();
  LOperand* value = UseRegister(instr->value());
  LStoreGlobal* result = new LStoreGlobal(value, temp);
  return AssignEnvironment(result);
} else {
  LOperand* value = UseRegisterAtStart(instr->value());
  return new LStoreGlobal(value, NULL);
}

[Mads Ager (chromium), 2011-01-27 08:33:36]

http://codereview.chromium.org/6306014/diff/5001/src/arm/lithium-arm.cc
File src/arm/lithium-arm.cc (right):

http://codereview.chromium.org/6306014/diff/5001/src/arm/lithium-arm.cc#newco...
src/arm/lithium-arm.cc:1605: LOperand* temp = instr->check_hole_value() ?
TempRegister() : NULL;
On 2011/01/27 07:56:07, Kevin Millikin wrote:
> OK, but since almost every line has to check the same flag, I'd split it into:
> 
> if (instr->check_hole_value()) {
>   LOperand* temp = TempRegister();
>   LOperand* value = UseRegister(instr->value());
>   LStoreGlobal* result = new LStoreGlobal(value, temp);
>   return AssignEnvironment(result);
> } else {
>   LOperand* value = UseRegisterAtStart(instr->value());
>   return new LStoreGlobal(value, NULL);
> }

Done.