Make bitfields only as wide as necessary for enums
clang now complains when a BitField for an enum is too wide.
We could suppress this, but it seems kind of useful from an
uninformed distance, so I made a few bitfields smaller instead.
(For AddressingMode, since its size is target-dependent, I added
an explicit underlying type to the enum instead, which suppresses
the diag on a per-enum basis.)
This is without any understanding of the code I'm touching.
Especially the change in v8-internal.h feels a bit risky to me.
Bug: chromium:1348574
Change-Id: I73395de593045036b72dadf4e3147b5f7e13c958
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3794708
Commit-Queue: Nico Weber <thakis@chromium.org>
Reviewed-by: Leszek Swirski <leszeks@chromium.org>
Reviewed-by: Hannes Payer <hpayer@chromium.org>
Auto-Submit: Nico Weber <thakis@chromium.org>
Cr-Commit-Position: refs/heads/main@{#82109}
diff --git a/include/v8-internal.h b/include/v8-internal.h
index a27f3a3..5fb0f2a 100644
--- a/include/v8-internal.h
+++ b/include/v8-internal.h
@@ -574,7 +574,7 @@
static const int kNodeClassIdOffset = 1 * kApiSystemPointerSize;
static const int kNodeFlagsOffset = 1 * kApiSystemPointerSize + 3;
- static const int kNodeStateMask = 0x7;
+ static const int kNodeStateMask = 0x3;
static const int kNodeStateIsWeakValue = 2;
static const int kFirstNonstringType = 0x80;
diff --git a/src/ast/ast.h b/src/ast/ast.h
index 32438f9..8473f7f 100644
--- a/src/ast/ast.h
+++ b/src/ast/ast.h
@@ -1006,7 +1006,7 @@
friend class AstNodeFactory;
friend Zone;
- using TypeField = Expression::NextBitField<Type, 4>;
+ using TypeField = Expression::NextBitField<Type, 3>;
Literal(int smi, int position) : Expression(position, kLiteral), smi_(smi) {
bit_field_ = TypeField::update(bit_field_, kSmi);
diff --git a/src/base/bit-field.h b/src/base/bit-field.h
index 9a66468..ccfc23a 100644
--- a/src/base/bit-field.h
+++ b/src/base/bit-field.h
@@ -40,6 +40,11 @@
static constexpr U kNumValues = U{1} << kSize;
// Value for the field with all bits set.
+ // If clang complains
+ // "constexpr variable 'kMax' must be initialized by a constant expression"
+ // on this line, then you're creating a BitField for an enum with more bits
+ // than needed for the enum values. Either reduce the BitField size,
+ // or give the enum an explicit underlying type.
static constexpr T kMax = static_cast<T>(kNumValues - 1);
template <class T2, int size2>
diff --git a/src/compiler/backend/instruction-codes.h b/src/compiler/backend/instruction-codes.h
index 1add351..2fe2cd1 100644
--- a/src/compiler/backend/instruction-codes.h
+++ b/src/compiler/backend/instruction-codes.h
@@ -198,7 +198,7 @@
V(None) \
TARGET_ADDRESSING_MODE_LIST(V)
-enum AddressingMode {
+enum AddressingMode : uint8_t {
#define DECLARE_ADDRESSING_MODE(Name) kMode_##Name,
ADDRESSING_MODE_LIST(DECLARE_ADDRESSING_MODE)
#undef DECLARE_ADDRESSING_MODE
@@ -309,7 +309,7 @@
// LaneSizeField and AccessModeField are helper types to encode/decode a lane
// size, an access mode, or both inside the overlapping MiscField.
using LaneSizeField = base::BitField<int, 22, 8>;
-using AccessModeField = base::BitField<MemoryAccessMode, 30, 2>;
+using AccessModeField = base::BitField<MemoryAccessMode, 30, 1>;
// TODO(turbofan): {HasMemoryAccessMode} is currently only used to guard
// decoding (in CodeGenerator and InstructionScheduler). Encoding (in
// InstructionSelector) is not yet guarded. There are in fact instructions for
diff --git a/src/compiler/backend/instruction.h b/src/compiler/backend/instruction.h
index bed43dc..64b9063 100644
--- a/src/compiler/backend/instruction.h
+++ b/src/compiler/backend/instruction.h
@@ -591,8 +591,8 @@
}
static_assert(KindField::kSize == 3);
- using LocationKindField = base::BitField64<LocationKind, 3, 2>;
- using RepresentationField = base::BitField64<MachineRepresentation, 5, 8>;
+ using LocationKindField = base::BitField64<LocationKind, 3, 1>;
+ using RepresentationField = LocationKindField::Next<MachineRepresentation, 8>;
using IndexField = base::BitField64<int32_t, 35, 29>;
};
diff --git a/src/handles/global-handles.cc b/src/handles/global-handles.cc
index 536059f..ae9e70b 100644
--- a/src/handles/global-handles.cc
+++ b/src/handles/global-handles.cc
@@ -575,7 +575,7 @@
// This stores three flags (independent, partially_dependent and
// in_young_list) and a State.
- using NodeState = base::BitField8<State, 0, 3>;
+ using NodeState = base::BitField8<State, 0, 2>;
// Tracks whether the node is contained in the set of young nodes. This bit
// persists across allocating and freeing a node as it's only cleaned up
// when young nodes are proccessed.
diff --git a/src/maglev/maglev-ir.h b/src/maglev/maglev-ir.h
index 95aadfb..f07f9fe 100644
--- a/src/maglev/maglev-ir.h
+++ b/src/maglev/maglev-ir.h
@@ -315,7 +315,7 @@
return kNeedsRegisterSnapshotBit::decode(bitfield_);
}
constexpr bool is_pure() const {
- return (bitfield_ | kPureMask) == kPureValue;
+ return (bitfield_ & kPureMask) == kPureValue;
}
constexpr bool is_required_when_unused() const {
return can_write() || non_memory_side_effects();
diff --git a/src/wasm/wasm-code-manager.h b/src/wasm/wasm-code-manager.h
index f832942..81c7cce 100644
--- a/src/wasm/wasm-code-manager.h
+++ b/src/wasm/wasm-code-manager.h
@@ -487,7 +487,7 @@
int trap_handler_index_ = -1;
// Bits encoded in {flags_}:
- using KindField = base::BitField8<Kind, 0, 3>;
+ using KindField = base::BitField8<Kind, 0, 2>;
using ExecutionTierField = KindField::Next<ExecutionTier, 2>;
using ForDebuggingField = ExecutionTierField::Next<ForDebugging, 2>;
