Skip to content

Conversation

@meg-gupta
Copy link
Contributor

No description provided.

@meg-gupta meg-gupta force-pushed the fixclosure branch 2 times, most recently from 96e02ec to bb752a6 Compare August 16, 2017 22:52
@meg-gupta
Copy link
Contributor Author

This bug came up in swb branch for test32:core\test\Bugs\randomBugs.js. I don't have a repro for without the flags -forceSoftwareWriteBarrier -verifyBarrierBit yet. It requires a specific pattern of register allocation to raise this bug. With the flags, the result of consecutive LdSlotArr s are assigned 2 different registers and peeps optimizes it incorrectly. Without the flags, am unable to repro such a scenario yet.

void
Peeps::SetReg(RegNum reg, StackSym *sym)
{
this->ClearReg(sym->scratch.peeps.reg);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this->ClearReg(sym->scratch.peeps.reg); [](start = 3, length = 40)

This should probably move bellow your new code.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right, will move this below.

Copy link
Collaborator

@LouisLaf LouisLaf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

@meg-gupta meg-gupta force-pushed the fixclosure branch 2 times, most recently from 45cd22a to 660fefc Compare August 21, 2017 19:17
Fixes OS#11093413

PeepAssign optimizes away when it says multiple reloads due to LdSlotArr, because these look like spill reloads.
We should avoid doing this for closure symbols.
@chakrabot chakrabot merged commit 95235ee into chakra-core:release/1.6 Aug 21, 2017
chakrabot pushed a commit that referenced this pull request Aug 21, 2017
Merge pull request #3536 from meg-gupta:fixclosure
chakrabot pushed a commit that referenced this pull request Aug 21, 2017
Merge pull request #3536 from meg-gupta:fixclosure
chakrabot pushed a commit that referenced this pull request Aug 21, 2017
…tack symbols

Merge pull request #3536 from meg-gupta:fixclosure
chakrabot pushed a commit that referenced this pull request Nov 6, 2017
Merge pull request #4145 from obastemur:exc_ccr

Enable a test that was previously disabled on ccrobot. #3536
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants