Disabling "enable-webassembly-baseline" does not really disable Liftoff |
||||
Issue description
In V8, we still have an ordering problem with processing flags. In this
case, the "--wasm-tier-up" flag implies "--liftoff", and since
"--wasm-tier-up" is enabled by default, also "--liftoff" is enabled by
default.
If if "enable-webassembly-baseline" is explicitly set to "disabled", we pass "--no-wasm-tier-up" via {SetFlagsFromString}, which correctly disables tier up, but since the implications were already applied before that, the "--liftoff" flag stays enabled.
Fix is here: https://crrev.com/c/1148386
This will have to be merged to M-69, requesting that right away. It's an extremely low-risk fix which should make it to the beta release.
Setting release blocker, as without this fix we cannot disable Liftoff via Finch, and we don't want to launch without that option.
Anyone with the power to do so, feel free to land the CL right away and merge it back to M-69 (via v8's merge process).
,
Jul 24
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/fab5510cbf8165739f2d228ba6d4c25c5b5530f8 commit fab5510cbf8165739f2d228ba6d4c25c5b5530f8 Author: Clemens Hammacher <clemensh@chromium.org> Date: Tue Jul 24 16:19:19 2018 [Flags] Explicitly disable Liftoff if tier up is disabled In V8, we still have an ordering problem with processing flags. In this case, the "--wasm-tier-up" flag implies "--liftoff", and since "--wasm-tier-up" is enabled by default, also "--liftoff" is enabled by default. If we now set "--no-wasm-tier-up" (via {SetFlagsFromString}), this does not disable "--liftoff". Hence, disable it explicitly. R=jochen@chromium.org, titzer@chromium.org Bug: chromium:866924 , chromium:787421 Change-Id: I298792bda07ef02a8f039c72be165ec815baa46f Reviewed-on: https://chromium-review.googlesource.com/1148386 Reviewed-by: Kentaro Hara <haraken@chromium.org> Commit-Queue: Clemens Hammacher <clemensh@chromium.org> Cr-Commit-Position: refs/heads/master@{#577571} [modify] https://crrev.com/fab5510cbf8165739f2d228ba6d4c25c5b5530f8/content/renderer/render_process_impl.cc
,
Jul 24
,
Jul 25
Your change meets the bar and is auto-approved for M69. Please go ahead and merge the CL to branch 3497 manually. Please contact milestone owner if you have questions. Owners: amineer@(Android), kariahda@(iOS), cindyb@(ChromeOS), govind@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Jul 25
Pls merge your change to M69 branch 3497 latest by 3:00 PM PT today, Wednesday (07/25/18). Thank you.
,
Jul 25
Backmerge CL here: https://crrev.com/c/1150380 Waiting for committer/owner LGTM.
,
Jul 26
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f48932797bc3447281a4d389f725362328cde84b commit f48932797bc3447281a4d389f725362328cde84b Author: Jochen Eisinger <jochen@chromium.org> Date: Thu Jul 26 07:50:09 2018 [Flags] Explicitly disable Liftoff if tier up is disabled In V8, we still have an ordering problem with processing flags. In this case, the "--wasm-tier-up" flag implies "--liftoff", and since "--wasm-tier-up" is enabled by default, also "--liftoff" is enabled by default. If we now set "--no-wasm-tier-up" (via {SetFlagsFromString}), this does not disable "--liftoff". Hence, disable it explicitly. R=jochen@chromium.org, titzer@chromium.org TBR=clemensh@chromium.org (cherry picked from commit fab5510cbf8165739f2d228ba6d4c25c5b5530f8) Bug: chromium:866924 , chromium:787421 Change-Id: I298792bda07ef02a8f039c72be165ec815baa46f Reviewed-on: https://chromium-review.googlesource.com/1148386 Reviewed-by: Kentaro Hara <haraken@chromium.org> Commit-Queue: Clemens Hammacher <clemensh@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#577571} Reviewed-on: https://chromium-review.googlesource.com/1150380 Reviewed-by: Jochen Eisinger <jochen@chromium.org> Cr-Commit-Position: refs/branch-heads/3497@{#101} Cr-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753} [modify] https://crrev.com/f48932797bc3447281a4d389f725362328cde84b/content/renderer/render_process_impl.cc |
||||
►
Sign in to add a comment |
||||
Comment 1 by clemensh@chromium.org
, Jul 24