Skip to content

Commit 42d8a06

Browse files
roberteg16claude
andcommitted
roofline: clarify storage-id / tensor-id comments
Address review: define "tensor id" (a ggml_tensor's address, dedup key) and "storage id" (the view_src-root buffer address, shared by aliasing tensors) at the op_record fields, and drop the vague word "facts" from the surrounding comments in favour of concrete wording. Co-Authored-By: Claude Opus 4 (1M context) <noreply@anthropic.com>
1 parent af6c190 commit 42d8a06

1 file changed

Lines changed: 18 additions & 12 deletions

File tree

ggml/src/ggml-cuda/ggml-cuda-roofline.cpp

Lines changed: 18 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -53,9 +53,15 @@ struct op_record {
5353
int64_t bytes = 0; // total HBM traffic: destination + all sources
5454
int64_t dst_bytes = 0; // ggml_nbytes(destination)
5555
int64_t src_bytes[GGML_MAX_SRC] = {}; // ggml_nbytes(source)
56-
uint64_t dst_sid = 0; // storage id (view_src root) of destination
57-
uint64_t src_ids[GGML_MAX_SRC] = {}; // source tensor id (dedup key)
58-
uint64_t src_sids[GGML_MAX_SRC] = {}; // source storage id (view_src root)
56+
// Two identities per tensor, so the consumer can tell which tensors actually move memory
57+
// without seeing the graph. A *tensor id* is one ggml_tensor's address (identifies a single
58+
// tensor; used to dedup a tensor read by several ops). A *storage id* is the address of the
59+
// tensor that owns the underlying buffer -- ggml's view_src root -- so tensors that alias the
60+
// same memory (a view and its source; an in-place op's output and the dst it was handed in as
61+
// a source) share one storage id; that is how the consumer spots internal and in-place tensors.
62+
uint64_t dst_sid = 0; // destination's storage id (its buffer)
63+
uint64_t src_ids[GGML_MAX_SRC] = {}; // each source's tensor id (dedup key)
64+
uint64_t src_sids[GGML_MAX_SRC] = {}; // each source's storage id (its buffer)
5965
int64_t M = 0, N = 0, K = 0, n_experts = 0, top_k = 0; // matmul dimensions
6066
std::vector<op_record> fused_nodes; // per-node geometry when this row is a fused group (else empty)
6167
};
@@ -261,11 +267,11 @@ void write_shape(std::ostringstream & out, const char * key, const int64_t ne[4]
261267
out << "\"" << key << "\": [" << ne[0] << ", " << ne[1] << ", " << ne[2] << ", " << ne[3] << "]";
262268
}
263269

264-
// One fused node's geometry plus the raw per-tensor facts the consumer needs to compute the
265-
// fused group's HBM traffic itself: destination/source byte counts, a storage id per tensor
266-
// (view_src root -- tells which tensors alias the same buffer), and a source tensor id (dedup
267-
// key). The consumer applies the exclusion policy (skip views, drop in-place destinations and
268-
// internal reads, dedup); the producer only reports facts.
270+
// One fused node: its geometry plus the raw per-tensor data the consumer needs to compute the
271+
// fused group's HBM traffic itself -- destination/source byte counts, a storage id per tensor
272+
// (which buffer it uses, so aliasing tensors are recognised) and a source tensor id (dedup key).
273+
// The consumer applies the exclusion policy (skip views, drop in-place destinations and internal
274+
// reads, dedup); the producer only records each tensor's size and identity.
269275
void write_fused_node(std::ostringstream & out, const op_record & rec) {
270276
out << "{\"ggml_op\": \"" << rec.op << "\", "
271277
<< "\"dtype\": \"" << rec.dtype << "\", \"quant\": \"" << rec.quant << "\", ";
@@ -510,13 +516,13 @@ void ggml_cuda_roofline_fuse_ops(const struct ggml_cgraph * cgraph, int node_idx
510516

511517
// Keep the head op's geometry (op name, shapes, M/N/K) so the row is still labelled by the
512518
// head op. The row-level byte fields stay head-only and are ignored for fused rows; the
513-
// consumer derives the group's traffic from the per-node facts recorded below.
519+
// consumer derives the group's traffic from the per-node data recorded below.
514520
op_record rec;
515521
fill_head_record(rec, head);
516522

517-
// Record every fused node's geometry plus its raw per-tensor byte/storage facts (captured by
518-
// fill_head_record while the graph is live). The consumer sums exact FLOPs across the group
519-
// and computes the group's external HBM traffic from these facts -- see write_fused_node.
523+
// Record every fused node's geometry plus its raw per-tensor byte counts and storage/tensor
524+
// ids (captured by fill_head_record while the graph is live). The consumer sums exact FLOPs
525+
// across the group and computes its external HBM traffic from them -- see write_fused_node.
520526
rec.fused_nodes.reserve(node_count);
521527
for (int j = node_idx; j < node_idx + node_count; ++j) {
522528
op_record sub;

0 commit comments

Comments
 (0)