[v12.x] deps: V8: backport 07ee86a5a28b#32619
Uh oh!
There was an error while loading. Please reload this page.
Conversation
Original commit message: PPC: allow for calling CFunctions without function descriptors on AIX. The calling conventions on AIX uses function descriptors, which means that pointers to functions do not point to code, but instead point to metadata about them. When calling JITed code, we must assure to use function descriptors instead of raw pointers when needed. Before this CL 213504b, all CallCFunction on AIX were guaranteed to have function descriptors. Starting form the CL mentioned above, CallCFunction can also Jump to a Trampoline which does not have a function descriptor, hence a new "CallCFunctionWithoutFunctionDescriptor" method is proposed to deal with this issue. BUG= v8:9766 Change-Id: I9343c31c812f5d4dda8503a5adf024b24dbde072 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1825961 Commit-Queue: Milad Farazmand <[email protected]> Reviewed-by: Michael Starzinger <[email protected]> Reviewed-by: Jakob Gruber <[email protected]> Cr-Commit-Position: refs/heads/master@{#64357} Refs: v8/v8@07ee86a
nodejs-github-bot added build 
targos changed the title 
nodejs-github-bot commented Apr 2, 2020
richardlau left a comment
richardlau left a comment There was a problem hiding this comment.
RSLGTM. I've added a commit to undo the skip of the --jitless test on AIX from #32594.
miladfarca commented Apr 2, 2020
@richardlau Could you please start another pr test |
nodejs-github-bot commented Apr 2, 2020
richardlau commented Apr 2, 2020
GitHub has been a bit unstable this evening. Will wait a bit for things to stabilise. |
miladfarca commented Apr 2, 2020
@richardlau sounds good, seems like one of my commits still hasn't showed up. |
| // Enums used by CEntry. | ||
| enum SaveFPRegsMode{kDontSaveFPRegs, kSaveFPRegs }; | ||
| enum ArgvMode{kArgvOnStack, kArgvInRegister }; | ||
| enum FunctionDescriptorMode{kNoFunctionDescriptor, kHasFunctionDescriptor }; |
There was a problem hiding this comment.
Shouldn't this have been enum FunctionDescriptorMode : bool{/* ... */ }, or (probably more idiomatic for V8) use the enum instead of bool? Relying on int-to-bool coercion looks a bit footgunny.
There was a problem hiding this comment.
I'm guessing they were ok with it as it's following the previous enums SaveFPRegsMode and ArgvMode.
There was a problem hiding this comment.
Right, but those are of type SaveFPRegsMode and ArgvMode in function parameter lists, there's no coercion to bool taking place.
There was a problem hiding this comment.
that's right however the int value 0 (kNoFunctionDescriptor) is implicitly translating to false at all cases, and true otherwise. You're right that marking it as bool could have made it more clear.
MylesBorins commented Apr 2, 2020
miladfarca commented Apr 2, 2020
Should be safe to run tests now, files are synced. |
This comment has been minimized.
This comment has been minimized.
miladfarca commented Apr 2, 2020
FYI, implementation under |
Original commit message: PPC: allow for calling CFunctions without function descriptors on AIX. The calling conventions on AIX uses function descriptors, which means that pointers to functions do not point to code, but instead point to metadata about them. When calling JITed code, we must assure to use function descriptors instead of raw pointers when needed. Before this CL 213504b, all CallCFunction on AIX were guaranteed to have function descriptors. Starting form the CL mentioned above, CallCFunction can also Jump to a Trampoline which does not have a function descriptor, hence a new "CallCFunctionWithoutFunctionDescriptor" method is proposed to deal with this issue. BUG= v8:9766 Change-Id: I9343c31c812f5d4dda8503a5adf024b24dbde072 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1825961 Commit-Queue: Milad Farazmand <[email protected]> Reviewed-by: Michael Starzinger <[email protected]> Reviewed-by: Jakob Gruber <[email protected]> Cr-Commit-Position: refs/heads/master@{#64357} Refs: v8/v8@07ee86a PR-URL: #32619 Refs: v8/v8@07ee86a Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]>
PR-URL: #32619 Refs: v8/v8@07ee86a Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]>
MylesBorins commented Apr 3, 2020
landed in e88960d...e685f12 |
6 participants
Original commit message:
Refs: v8/v8@07ee86a
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passes