|
|
Chromium Code Reviews|
Created:
4 years, 4 months ago by Toon Verwaest Modified:
4 years, 4 months ago CC:
v8-reviews_googlegroups.com Base URL:
https://chromium.googlesource.com/v8/v8.git@master Target Ref:
refs/pending/heads/master Project:
v8 Visibility:
Public. |
DescriptionBetter pack fields in Variable
This reduces sizeof(Variable) from 64 to 40 on x64
BUG=v8:5209
Committed: https://crrev.com/d84343568047c8621a6b8f88f20a7f34586321b8
Cr-Commit-Position: refs/heads/master@{#38659}
Patch Set 1 #
Messages
Total messages: 17 (7 generated)
verwaest@chromium.org changed reviewers: + marja@chromium.org
ptal
lgtm
The CQ bit was checked by verwaest@chromium.org
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
Try jobs failed on following builders: v8_android_arm_compile_rel on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_android_arm_compile_rel/...) v8_linux64_asan_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_asan_rel_ng/buil...) v8_linux64_avx2_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_avx2_rel_ng/buil...) v8_linux64_gyp_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_gyp_rel_ng/build...) v8_linux64_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_rel_ng/builds/10984) 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/...) v8_linux_dbg_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux_dbg_ng/builds/10976) v8_linux_gcc_compile_rel on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux_gcc_compile_rel/bu...) v8_linux_mips64el_compile_rel on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux_mips64el_compile_r...) v8_linux_mipsel_compile_rel on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux_mipsel_compile_rel...) v8_linux_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux_rel_ng/builds/10922) v8_presubmit on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_presubmit/builds/21825)
The CQ bit was checked by verwaest@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Committed patchset #1 (id:1)
Message was sent while issue was closed.
Description was changed from ========== Better pack fields in Variable This reduces sizeof(Variable) from 64 to 40 on x64 BUG=v8:5209 ========== to ========== Better pack fields in Variable This reduces sizeof(Variable) from 64 to 40 on x64 BUG=v8:5209 Committed: https://crrev.com/d84343568047c8621a6b8f88f20a7f34586321b8 Cr-Commit-Position: refs/heads/master@{#38659} ==========
Message was sent while issue was closed.
Patchset 1 (id:??) landed as https://crrev.com/d84343568047c8621a6b8f88f20a7f34586321b8 Cr-Commit-Position: refs/heads/master@{#38659}
Message was sent while issue was closed.
jkummerow@chromium.org changed reviewers: + jkummerow@chromium.org
Message was sent while issue was closed.
This seems to have broken compilation on the GCC based ARM and MIPS bots, e.g.:
/mnt/data/b/build/slave/V8_Mips_-_builder/build/mips-mti-linux-gnu/2015.01-7/bin/mips-mti-linux-gnu-g++
-MMD -MF obj/src/v8_base.allocation-site-scopes.o.d -DCR_CLANG_REVISION=277962-1
-DV8_TARGET_ARCH_MIPS -DCAN_USE_FPU_INSTRUCTIONS -D__mips_hard_float=1
-DV8_DEPRECATION_WARNINGS -DV8_IMMINENT_DEPRECATION_WARNINGS -D_MIPS_TARGET_HW
-D_MIPS_ARCH_MIPS32R2 -DFPU_MODE_FP32 -DENABLE_HANDLE_ZAPPING -I../.. -Igen
-I../../include -Wall -Werror -Wno-unused-parameter -pthread
-Wmissing-field-initializers -fvisibility=hidden -Wno-sign-compare -EB
-Wno-error=array-bounds -mhard-float -mips32r2 -mfp32 -Wa,-mips32r2 -EB
-Wno-error=array-bounds -mhard-float -mips32r2 -mfp32 -Wa,-mips32r2
-fdata-sections -ffunction-sections -O3 -fdata-sections -ffunction-sections -O3
-Wnon-virtual-dtor -fno-exceptions -fno-rtti -std=gnu++11 -c
../../src/allocation-site-scopes.cc -o obj/src/v8_base.allocation-site-scopes.o
In file included from ../../src/ast/ast.h:10:0,
from ../../src/allocation-site-scopes.h:8,
from ../../src/allocation-site-scopes.cc:5:
../../src/ast/variables.h:123:24: error: ‘v8::internal::Variable::mode_’ is too
small to hold all values of ‘enum v8::internal::VariableMode’ [-Werror]
VariableMode mode_ : 3;
^
../../src/ast/variables.h:125:16: error: ‘v8::internal::Variable::kind_’ is too
small to hold all values of ‘enum v8::internal::Variable::Kind’ [-Werror]
Kind kind_ : 2;
^
../../src/ast/variables.h:128:32: error: ‘v8::internal::Variable::location_’ is
too small to hold all values of ‘enum class v8::internal::VariableLocation’
[-Werror]
VariableLocation location_ : 3;
^
../../src/ast/variables.h:133:45: error:
‘v8::internal::Variable::initialization_flag_’ is too small to hold all values
of ‘enum v8::internal::InitializationFlag’ [-Werror]
InitializationFlag initialization_flag_ : 2;
^
../../src/ast/variables.h:134:39: error:
‘v8::internal::Variable::maybe_assigned_’ is too small to hold all values of
‘enum v8::internal::MaybeAssignedFlag’ [-Werror]
MaybeAssignedFlag maybe_assigned_ : 2;
^
cc1plus: all warnings being treated as errors
Message was sent while issue was closed.
A revert of this CL (patchset #1 id:1) has been created in https://codereview.chromium.org/2249203002/ by hablich@chromium.org. The reason for reverting is: Revert: Breaks ARM build: https://uberchromegw.corp.google.com/i/client.v8.ports/builders/V8%20Arm%20-%....
Message was sent while issue was closed.
Thanks for noticing + revert. I guess we'll just have to resort to BitField here then.
Message was sent while issue was closed.
Description was changed from ========== Better pack fields in Variable This reduces sizeof(Variable) from 64 to 40 on x64 BUG=v8:5209 Committed: https://crrev.com/d84343568047c8621a6b8f88f20a7f34586321b8 Cr-Commit-Position: refs/heads/master@{#38659} ========== to ========== Better pack fields in Variable This reduces sizeof(Variable) from 64 to 40 on x64 BUG=v8:5209 Committed: https://crrev.com/d84343568047c8621a6b8f88f20a7f34586321b8 Cr-Commit-Position: refs/heads/master@{#38659} ========== |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
