Skip to content

Conversation

@rajatd
Copy link
Contributor

@rajatd rajatd commented Jan 5, 2017

For initializing a nested function object on the stack, the jitted code was using hardcoded field values and offsets from the function's functionProxy. This is not legal with redeferral as the proxy could have been a full function body and could be redeferred from the time its enclosing function was jitted to when it was executed. Redeferral will throw away the existing function body and reallocate a parseable function info which will then be set as function proxy on the function info. This reallocation means that the hardcoded values and offsets in the jitted code are no longer valid.

Fix is to not use compile time values but to jit indirections. We already do that for oop jit, just made in-proc jit also do the same.


This change is Reviewable

}
}

functionInfoOpnd = IR::IndirOpnd::New(functionProxyOpnd->AsRegOpnd(), Js::FunctionProxy::GetOffsetOfFunctionInfo(), TyMachReg, func);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@pleath - i was in two minds whether to directly use the nestedInfo for initializing functionInfo. I'm sure I have asked you this before, but do we expect functionInfo to be different from functionInfo->functionBodyImpl->functionInfo in some cases?

Copy link
Contributor

Choose a reason for hiding this comment

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

No. Never. Once we allocate a FunctionInfo for a given user function, it never changes. So, yes, once we've loaded it from the nested array we can just reuse it throughout the code sequence.

{
if (functionProxyOpnd->IsRegOpnd())
{
functionInfoOpnd = IR::IndirOpnd::New(functionProxyOpnd->AsRegOpnd(), Js::FunctionProxy::GetOffsetOfFunctionInfo(), TyMachReg, func);
Copy link
Collaborator

Choose a reason for hiding this comment

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

functionInfoOpnd [](start = 12, length = 16)

I am also wondering why we need re-check functionInfo from functionProxy, should it be the same?

@rajatd
Copy link
Contributor Author

rajatd commented Jan 6, 2017

@pleath please review

@dilijev
Copy link
Contributor

dilijev commented Jan 6, 2017

You can now resolve the CI issues as described here: #2332 (comment)

@dilijev
Copy link
Contributor

dilijev commented Feb 2, 2017

@dotnet-bot test this please

@pleath
Copy link
Contributor

pleath commented Feb 10, 2017

Change checked in under another PR.

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