Skip to content

Commit fab3ba3

Browse files
DadaIsCrazyV8 LUCI CQ
DadaIsCrazy
authored and
V8 LUCI CQ
committed
[turboshaft] Compute correct input_count in CreateOperation
Fixed: 331241020 Bug: v8:12783 Change-Id: I30d5a1ebc7e10313fc95c42b31c065c35f95cc91 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5400799 Commit-Queue: Matthias Liedtke <[email protected]> Reviewed-by: Nico Hartmann <[email protected]> Auto-Submit: Darius Mercadier <[email protected]> Reviewed-by: Matthias Liedtke <[email protected]> Cr-Commit-Position: refs/heads/main@{#93065}
1 parent 661f055 commit fab3ba3

File tree

3 files changed

+77
-11
lines changed

3 files changed

+77
-11
lines changed

‎src/compiler/turboshaft/index.h‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,10 +38,19 @@ class ConstOrV;
3838
// Compared to `Operation*`, it is more memory efficient (32bit) and stable when
3939
// the operations buffer is re-allocated.
4040
class OpIndex {
41-
public:
41+
protected:
42+
// We make this constructor protected so that integers are not easily
43+
// convertible to OpIndex. FromOffset should be used instead to create an
44+
// OpIndex from an offset.
4245
explicit constexpr OpIndex(uint32_t offset) : offset_(offset) {
4346
DCHECK(CheckInvariants());
4447
}
48+
friend class OperationBuffer;
49+
50+
public:
51+
static constexpr OpIndex FromOffset(uint32_t offset) {
52+
return OpIndex(offset);
53+
}
4554
constexpr OpIndex() : offset_(std::numeric_limits<uint32_t>::max()) {}
4655
template <typename T, typename C>
4756
OpIndex(const ConstOrV<T, C>&) { // NOLINT(runtime/explicit)

‎src/compiler/turboshaft/operations.h‎

Lines changed: 65 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -8405,15 +8405,66 @@ V8_EXPORT_PRIVATE V8_INLINE bool ShouldSkipOperation(const Operation& op) {
84058405
}
84068406

84078407
namespace detail {
8408-
// Computes the number of inputs of an operation, ignoring non-OpIndex inputs
8409-
// (which are always inlined in the operation) and `base::Vector<OpIndex>`
8410-
// inputs.
8411-
template <typename T>
8408+
// Defining `input_count` to compute the number of OpIndex inputs of an
8409+
// operation.
8410+
8411+
// There is one overload for each possible type of parameters for all
8412+
// Operations rather than a default generic overload, so that we don't
8413+
// accidentally forget some types (eg, if a new Operation takes its inputs as a
8414+
// std::vector<OpIndex>, we shouldn't count this as "0 inputs because it's
8415+
// neither raw OpIndex nor base::Vector<OpIndex>", which a generic overload
8416+
// might do).
8417+
8418+
// Base case
8419+
constexpr size_t input_count() { return 0; }
8420+
8421+
// All parameters that are not OpIndex and should thus not count towards the
8422+
// "input_count" of the operations.
8423+
template <typename T, typename = std::enable_if_t<std::is_enum_v<T> ||
8424+
std::is_integral_v<T> ||
8425+
std::is_floating_point_v<T>>>
84128426
constexpr size_t input_count(T) {
84138427
return 0;
84148428
}
8415-
constexpr size_t input_count() { return 0; }
8429+
template <typename T>
8430+
constexpr size_t input_count(const MaybeHandle<T>) {
8431+
return 0;
8432+
}
8433+
template <typename T>
8434+
constexpr size_t input_count(const Handle<T>) {
8435+
return 0;
8436+
}
8437+
template <typename T>
8438+
constexpr size_t input_count(const base::Flags<T>) {
8439+
return 0;
8440+
}
8441+
constexpr size_t input_count(const Block*) { return 0; }
8442+
constexpr size_t input_count(const TSCallDescriptor*) { return 0; }
8443+
constexpr size_t input_count(const char*) { return 0; }
8444+
constexpr size_t input_count(const DeoptimizeParameters*) { return 0; }
8445+
constexpr size_t input_count(const FastApiCallParameters*) { return 0; }
8446+
constexpr size_t input_count(const FrameStateData*) { return 0; }
8447+
constexpr size_t input_count(const base::Vector<SwitchOp::Case>) { return 0; }
8448+
constexpr size_t input_count(LoadOp::Kind) { return 0; }
8449+
constexpr size_t input_count(RegisterRepresentation) { return 0; }
8450+
constexpr size_t input_count(MemoryRepresentation) { return 0; }
8451+
constexpr size_t input_count(OpEffects) { return 0; }
8452+
inline size_t input_count(const ElementsTransition) { return 0; }
8453+
inline size_t input_count(const FeedbackSource) { return 0; }
8454+
inline size_t input_count(const ZoneRefSet<Map>) { return 0; }
8455+
inline size_t input_count(ConstantOp::Storage) { return 0; }
8456+
inline size_t input_count(Type) { return 0; }
8457+
#ifdef V8_ENABLE_WEBASSEMBLY
8458+
constexpr size_t input_count(const wasm::WasmGlobal*) { return 0; }
8459+
constexpr size_t input_count(const wasm::StructType*) { return 0; }
8460+
constexpr size_t input_count(const wasm::ArrayType*) { return 0; }
8461+
constexpr size_t input_count(wasm::ValueType) { return 0; }
8462+
constexpr size_t input_count(WasmTypeCheckConfig) { return 0; }
8463+
#endif
8464+
8465+
// All parameters that are OpIndex-like (ie, OpIndex, and OpIndex containers)
84168466
constexpr size_t input_count(OpIndex) { return 1; }
8467+
constexpr size_t input_count(OptionalOpIndex) { return 1; }
84178468
constexpr size_t input_count(base::Vector<const OpIndex> inputs) {
84188469
return inputs.size();
84198470
}
@@ -8422,10 +8473,16 @@ constexpr size_t input_count(base::Vector<const OpIndex> inputs) {
84228473
template <typename Op, typename... Args>
84238474
Op* CreateOperation(base::SmallVector<OperationStorageSlot, 32>& storage,
84248475
Args... args) {
8425-
size_t size = Operation::StorageSlotCount(
8426-
Op::opcode, (0 + ... + detail::input_count(args)));
8476+
size_t input_count = (0 + ... + detail::input_count(args));
8477+
size_t size = Operation::StorageSlotCount(Op::opcode, input_count);
84278478
storage.resize_no_init(size);
8428-
return new (storage.data()) Op(args...);
8479+
Op* op = new (storage.data()) Op(args...);
8480+
// Checking that the {input_count} we computed is at least the actual
8481+
// input_count of the operation. {input_count} could be greater in the case of
8482+
// OptionalOpIndex: they count for 1 input when computing {input_count} here,
8483+
// but in Operations, they only count for 1 input when they are valid.
8484+
DCHECK_GE(input_count, op->input_count);
8485+
return op;
84298486
}
84308487

84318488
} // namespace v8::internal::compiler::turboshaft

‎src/wasm/turboshaft-graph-interface.cc‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7269,8 +7269,8 @@ class TurboshaftGraphBuildingInterface : public WasmGraphBuilderBase {
72697269
using InliningIdField = PositionField::Next<uint8_t, kInliningIdFieldSize>;
72707270

72717271
OpIndex WasmPositionToOpIndex(WasmCodePosition position, int inlining_id) {
7272-
return OpIndex(PositionField::encode(position) |
7273-
InliningIdField::encode(inlining_id));
7272+
return OpIndex::FromOffset(PositionField::encode(position) |
7273+
InliningIdField::encode(inlining_id));
72747274
}
72757275

72767276
SourcePosition OpIndexToSourcePosition(OpIndex index) {

0 commit comments

Comments
 (0)