Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2594543003:
[Interpreter] Ensure that a function is compiled before tiering up to baseline. (Closed)
Description[Interpreter] Ensure that a function is compiled before tiering up to baseline.
When baselining a function using the BaselineFunctionOnNextCall intrinsic, it is
not always ensured that a function is already compiled. Update the
Runtime_BaselineFunctionOnNextCall function to trigger a compile if it is not already
compiled.
BUG=v8:5768
Review-Url: https://codereview.chromium.org/2594543003
Cr-Commit-Position: refs/heads/master@{#42033}
Committed: https://chromium.googlesource.com/v8/v8/+/d338b94e869a845a53afd5e057c807ed2e2b3a6f
Patch Set 1 #Patch Set 2 : Changed it to a DCHECK to ensure we always compile before baselining. #Patch Set 3 : Updated runtime function to call Compile if the function is not already compiled. #Patch Set 4 : Fixed an error in DCHECK. #
Messages
Total messages: 29 (21 generated)
The CQ bit was checked by mythria@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
mythria@chromium.org changed reviewers: + bmeurer@chromium.org, leszeks@chromium.org, rmcilroy@chromium.org
We found this bug when debugging this bug: https://bugs.chromium.org/p/v8/issues/detail?id=5758. When we move to Baseline even before we have compiled the function, feedback metadata will not be created. I think this would be a issue only when using %BaselineFunctionOnNextCall. PTAL. Thanks, Mythri
LGTM, thanks!
On 2016/12/21 11:54:46, Benedikt Meurer wrote: > LGTM, thanks! I'm not sure this is they way we should be fixing this, I think we want to maintain the invariant that a function is compiled before being baselined. I added a comment to the bug
I changed it to a DCHECK to ensure a function is compiled before trying to baseline the code. PTAL.
The CQ bit was checked by mythria@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: v8_linux64_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_rel_ng/builds/18499) v8_linux_arm64_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm64_rel_ng/build...) v8_linux_arm_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm_rel_ng/builds/...)
Description was changed from ========== [Interpreter] Moving a funciton to baseline should create feedback metadata if required. When we switch a function to baseline and if the function was not compiled earlier, the feedback metadata would not be created. So when switching a function to baseline, we have to call EnsureFeedbackMetadata to create the metadata if required. BUG=v8:5768 ========== to ========== [Interpreter] Ensure that a function is compiled before tiering up to baseline. When baselining a function using the BaselineFunctionOnNextCall intrinsic, it is not always ensured that a function is already compiled. Update the Runtime_BaselineFunctionOnNextCall function to trigger a compile if it is not already compiled. BUG=v8:5768 ==========
The CQ bit was checked by mythria@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== [Interpreter] Ensure that a function is compiled before tiering up to baseline. When baselining a function using the BaselineFunctionOnNextCall intrinsic, it is not always ensured that a function is already compiled. Update the Runtime_BaselineFunctionOnNextCall function to trigger a compile if it is not already compiled. BUG=v8:5768 ========== to ========== [Interpreter] Ensure that a function is compiled before tiering up to baseline. When baselining a function using the BaselineFunctionOnNextCall intrinsic, it is not always ensured that a function is already compiled. Update the Runtime_BaselineFunctionOnNextCall function to trigger a compile if it is not already compiled. BUG=v8:5768 ==========
Description was changed from ========== [Interpreter] Ensure that a function is compiled before tiering up to baseline. When baselining a function using the BaselineFunctionOnNextCall intrinsic, it is not always ensured that a function is already compiled. Update the Runtime_BaselineFunctionOnNextCall function to trigger a compile if it is not already compiled. BUG=v8:5768 ========== to ========== [Interpreter] Ensure that a function is compiled before tiering up to baseline. When baselining a function using the BaselineFunctionOnNextCall intrinsic, it is not always ensured that a function is already compiled. Update the Runtime_BaselineFunctionOnNextCall function to trigger a compile if it is not already compiled. BUG=v8:5768 ==========
Description was changed from ========== [Interpreter] Ensure that a function is compiled before tiering up to baseline. When baselining a function using the BaselineFunctionOnNextCall intrinsic, it is not always ensured that a function is already compiled. Update the Runtime_BaselineFunctionOnNextCall function to trigger a compile if it is not already compiled. BUG=v8:5768 ========== to ========== [Interpreter] Ensure that a function is compiled before tiering up to baseline. When baselining a function using the BaselineFunctionOnNextCall intrinsic, it is not always ensured that a function is already compiled. Update the Runtime_BaselineFunctionOnNextCall function to trigger a compile if it is not already compiled. BUG=v8:5768 ==========
I updated the runtime_BaselineFunctionOnNextCall to compile if the function is not already compiled. PTAL. Thanks, Mythri
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
LGTM, thanks.
The CQ bit was checked by mythria@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from bmeurer@chromium.org Link to the patchset: https://codereview.chromium.org/2594543003/#ps60001 (title: "Fixed an error in DCHECK.")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 60001, "attempt_start_ts": 1483456185012930,
"parent_rev": "a11a9c7dd7a993cfea6e404aaecbbc8a5f411021", "commit_rev":
"d338b94e869a845a53afd5e057c807ed2e2b3a6f"}
Message was sent while issue was closed.
Description was changed from ========== [Interpreter] Ensure that a function is compiled before tiering up to baseline. When baselining a function using the BaselineFunctionOnNextCall intrinsic, it is not always ensured that a function is already compiled. Update the Runtime_BaselineFunctionOnNextCall function to trigger a compile if it is not already compiled. BUG=v8:5768 ========== to ========== [Interpreter] Ensure that a function is compiled before tiering up to baseline. When baselining a function using the BaselineFunctionOnNextCall intrinsic, it is not always ensured that a function is already compiled. Update the Runtime_BaselineFunctionOnNextCall function to trigger a compile if it is not already compiled. BUG=v8:5768 Review-Url: https://codereview.chromium.org/2594543003 Cr-Commit-Position: refs/heads/master@{#42033} Committed: https://chromium.googlesource.com/v8/v8/+/d338b94e869a845a53afd5e057c807ed2e2... ==========
Message was sent while issue was closed.
Committed patchset #4 (id:60001) as https://chromium.googlesource.com/v8/v8/+/d338b94e869a845a53afd5e057c807ed2e2... |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
