Just refactoring to remove some line (hopefully) - #528
Open
TApplencourt wants to merge 26 commits into
Open
Conversation
opencl_tracepoints.rb defined its own class LTTng with duplicate print_tracepoint/print_enum logic, parallel to the shared utils/LTTng.rb module used by every other (AST-driven) backend. Since a bare `class LTTng` cannot coexist with utils/LTTng.rb's `module LTTng` (Ruby raises TypeError: LTTng is not a class), rename opencl's local tuple-parsing helper to `module LTTngFieldTuple` and drop its now-redundant print_tracepoint/print_enum, routing gen_opencl_probes.rb's two call sites through the shared LTTng.print_tracepoint instead. Also drop opencl_model.rb's duplicate MEMBER_SEPARATOR constant now that utils/LTTng.rb (required transitively via opencl_tracepoints.rb) defines the same value, eliminating a "already initialized constant" warning. Verified byte-identical against devel for opencl_tracepoints.tp, opencl_profiling.tp, opencl_model.yaml, btx_cl_model.yaml, and the remaining opencl_*.tp files, via direct diff and via utils/test_compare_generated_file.py (THAPI_FILTER=opencl). gen_babeltrace_cl_model.rb's parse_field was left unmerged with utils/gen_babeltrace_model_helper.rb's gen_bt_field_model: the latter depends on $types_by_name/$all_enum_names/$all_bitfield_names/ $all_struct_names/$integer_sizes/$integer_signed, all populated only by each AST backend's gen_*_library_base.rb from a parsed C AST. Opencl has no such AST pass (it parses cl.xml into OPENCL_MODEL instead), so building an equivalent global-population pipeline just to reuse gen_bt_field_model would be a large, unverifiable-by-byte-diff architectural addition. Left as documented partial unification (the existing shared gen_yaml call is retained). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
utils/test_compare_generated_file.py compared opencl_profiling.tp but not opencl_tracepoints.tp, the main opencl tracepoint output. Add it to the opencl file list so CI's check-same-generated-files job actually covers the file the opencl unification refactor touches. Verified: rerunning pytest with THAPI_FILTER=opencl against a clean devel baseline build and this branch's build still passes (2 passed, empty DeepDiff). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
OpenCL was the outlier with only opencl_tracepoints.tp/opencl_profiling.tp listed. Mirror the other backends (which guard their btx_*_model.yaml plus every *.tp) by adding the remaining opencl generated artifacts: tracer_opencl.c, btx_cl_model.yaml, opencl_model.yaml, and the arguments/build/devices/dump/source tracepoint files. All are byte-identical between devel and the opencl-unify refactor, so this only widens coverage. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Extract the six type-classification facts gen_bt_field_model reads from loose globals ($types_by_name, $all_enum_names, $all_bitfield_names, $all_struct_names, $integer_sizes, $integer_signed) plus to_scoped_class_name into an immutable TypeRegistry value object with integer_size/integer_signed? methods. A single global $type_registry is populated exactly as before; the top-level integer_size/integer_signed? helpers now delegate to it. No behaviour change: generated files remain byte-identical. New file utils/type_registry.rb wired into utils/Makefile.am EXTRA_DIST (verified via make distcheck). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Change gen_bt_field_model's signature to take a TypeRegistry as its first argument and read types_by_name / enum_names / bitfield_names / struct_names / integer_size / integer_signed? / class_namer from it instead of the loose globals and the top-level integer_size/integer_signed? helpers. The two internal call sites (gen_event_fields_bt_model, gen_extra_event_fields_bt_model) pass the global $type_registry, still the sole instance. No behaviour change: generated files remain byte-identical. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…als (AST backends) Add TypeRegistry.from_ast, which derives the integer-size/-sign lookups and the by-name type index from a backend's parsed AST (the enum/bitfield/struct name lists are still classified per-backend, since their rules differ). Each AST backend's gen_babeltrace_<x>_model.rb now constructs its own registry and passes it as the first argument through gen_event_bt_model / gen_extra_event_bt_model / gen_event_fields_bt_model / gen_bt_field_model. This removes ALL load-time side effects from gen_babeltrace_model_helper.rb: the $integer_sizes/$integer_signed/$types_by_name/$type_registry globals and the free integer_size/integer_signed? helpers are gone; requiring the helper now only defines functions. State is produced only by the explicit from_ast call that returns it. Generated files remain byte-identical for all backends. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…-time code gen_babeltrace_cl_model.rb defined empty INT_SIZE_MAP/INT_SIGN_MAP/$all_enums/ $all_types placeholders solely to satisfy gen_babeltrace_model_helper.rb's top-level load-time code (which read those globals to build $integer_sizes / $types_by_name). That load-time code was deleted when the helper moved to an explicitly-threaded TypeRegistry, so the placeholders are now dead. Removing them leaves opencl's generated btx_cl_model.yaml byte-identical. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The babeltrace-model de-globalization unified the six AST backends onto the shared gen_bt_field_model, but opencl's parse_field is deliberately left separate. Its de-globalization goal is already met (side-effect-free, reads OPENCL_MODEL explicitly), and merging it could not stay byte-identical: ctf_enum emits real CTF enumerations (AST backends have no such case), the input shape is a flat hash vs AST + LTTng objects, and pointer/signedness is encoded differently (bare-type signedness + explicit pointer flag vs a `*` in the type string whose rule forces unsigned). Comment records this as a reasoned decision so the next reader does not re-attempt it. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ng reader Each AST model generator now asserts, right after building its TypeRegistry, whether the backend has bitfield types. Empirically (loading each gen_<x>_library_base and inspecting the populated name list) the runtime invariant is: cuda, hip, mpi and itt have zero bitfield types; ze (148) and omp (12) have them. Note itt's .push line exists but sits in a conditional that never fires for its actual types, so itt asserts empty -- the initial non-empty assertion failed the itt build, confirming the real invariant. Also documents the remaining $all_bitfield_names read in gen_babeltrace_lib_helper.rb: it is in the library/bindings path (not the de-globalized model path), runs after the caller has required its gen_<x>_library_base, and cannot be dropped while gen_library_base.rb still reads $all_struct_names. All 53 oracle files remain byte-identical. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…d scaffolding) Cherry-pick of devel 2614b7a onto opencl-unify-refactor, resolving the opencl_model.rb conflict: this branch keeps MEMBER_SEPARATOR in utils/LTTng.rb rather than inline, so only the dead GENERATE_ENUMS_TRACEPOINTS constant, the enum-tracepoint block it gated, and the dead early-return are removed. Removed content is byte-identical to 2614b7a on all 8 files (164 deletions total). Byte-identical-safe: GENERATE_ENUMS_TRACEPOINTS was permanently false, so the gated blocks never executed and no generated output changes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…aders After 2614b7a deleted the GENERATE_ENUMS_TRACEPOINTS block, ENUMS, ENUM_PARAM_NAME_MAP and ENUM_TYPES have no populator and stay empty forever. Remove the three declarations, the always-false `ENUM_TYPES.include?` guard in lttng_in_type, and unwrap both `if ENUM_PARAM_NAME_MAP[name]` branches in In/OutScalar (the enum branch could never be taken; keep the else body). Byte-identical-safe: every removed reader was dead (empty-collection lookups), so no generated output changes. Verified: 53/53 oracle cases identical to the devel baseline. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Every AST backend (cuda/hip/mpi/omp/itt/ze) reimplemented the same loop to
split typedef'd types into enum/bitfield/struct/union name lists, then derived
the `_flags_t` bitfield aliases. Replace all six with one call to a shared
classify_ast_types in utils/gen_library_base.rb.
Unify the bitfield predicate on name.end_with?('flag_t'). ze previously scanned
enum members for a ZE_BIT value expression; this is provably equivalent on the
built APIs (ze: 74 bitfields either way, 0 diff in both directions; cuda/hip/mpi/
itt: 0; omp: 6 -> 12 after _flags_t derivation), so no per-backend lambda is
needed and the ZE_BIT scan is removed. The now-unused empty-array initializers
for the four name-list globals are dropped (classify_ast_types returns them).
Also removes the dead `$all_enums.find { ... }` whose result cuda/hip/mpi
discarded.
Byte-identical: 53/53 oracle cases identical to the devel baseline.
Net -71 lines (-115 across backends, +32 shared fn + call sites).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Every AST backend repeated the same 6-line block per meta-parameter YAML: load the file, iterate meta_parameters, const_get the type, register. Fold that into a single load_meta_parameters(filename) helper in utils/command.rb and replace all 12 call sites (hip, mpi, omp, itt, cuda x2, cudart, ze x5). Byte-identical: oracle 53/53. opencl left untouched (its own Command class, does not require utils/command.rb). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The identical 3-line upper_snake_case helper was copy-pasted into 7 backend model files (itt, cuda, cudart, hip, ze, ompt, opencl). Define it once at the top of utils/LTTng.rb, which every backend already requires, and drop the duplicates. mpi has its own distinct underscore() and is untouched. Byte-identical: oracle 53/53. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
All 6 AST gen_babeltrace_<x>_model.rb files repeated the identical 5-line TypeRegistry.from_ast(...) call reading the same globals, followed by a per-backend bitfield-presence assertion. Fold both into build_ast_registry( backend, expect_bitfields:) in gen_babeltrace_model_helper.rb; the invariant (ze/omp have bitfields, cuda/hip/mpi/itt do not) is preserved via the flag. Byte-identical: oracle 53/53. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
All 6 AST gen_babeltrace_<x>_model.rb files repeated the same two loops: one building [start, stop] event pairs per command (or a single event for the itt/omp phased:false case), and one building the extra events declared in <x>_events.yaml. Extract gen_command_events_bt_model(registry, provider_commands, phased:) and gen_extra_events_bt_model(registry, filename) into gen_babeltrace_model_helper.rb and collapse the call sites. Byte-identical: oracle 53/53. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
All 6 AST gen_<x>_library_base.rb files derived $objects (pointer-to-struct typedefs plus CustomType aliases of OBJECT_TYPES) and $int_scalars (typedefs aliasing integer types) with the same two loops. Extract find_objects(all_types, extra:) and find_int_scalars(all_types) into utils/gen_library_base.rb; hip's one seeded name is threaded via extra:. Unlike load_meta_parameters these helpers RETURN their result (assigned explicitly at the call site) rather than mutating a global. Byte-identical: oracle 53/53. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
cuda, hip and mpi each carried a byte-identical 35-line copy of the Handle/UUID to_s modules inside their library heredoc. Extract the one copy into print_handle_uuid_modules and call it from the three backends. ze keeps its own copy: it uses :data/:id field names and prints the UUID bytes back to front, so it cannot share this helper as-is. Generated output is byte-identical (oracle: 53 passed). Net -55 LOC.
cuda, hip, mpi, itt and ze each defined print_<x>_object(object) whose whole body was print_object(object). Call the shared helper directly. Generated output is byte-identical (oracle: 53 passed).
The per-backend print_union (all five backends) and mpi's print_struct only re-bound the namespace argument. Pass the namespace at the call site instead; the indirection hid which namespace was in play. Backends that add real behaviour on top of the shared helper (cuda/hip/ze UUID prepends, itt's function-pointer rewriting, ze's version enums) keep their wrappers. Generated output is byte-identical (oracle: 53 passed).
Same pass-through shape as print_union: the wrapper only supplied the namespace. itt, omp and ze keep theirs -- those pick between enum and bitfield printing, or handle version enums. Generated output is byte-identical (oracle: 53 passed).
cuda and hip both prepended the UUID module for structs whose class name mentions UUID, differing only in namespace. Hoist to print_struct_prepending_uuid in gen_library_base. ze keeps its own: it also selects the KUUID module for kernel UUIDs. Generated output is byte-identical (oracle: 53 passed).
CI lints every changed .rb with `rubocop --display-only-safe-correctable`, which fails on any autocorrectable offence. Five came from earlier commits on this branch (argument alignment, a long line, a trailing comma, a while-modifier); the indentation one in gen_babeltrace_lib_helper.rb pre-dates the branch but CI lints that file because we touch it. Applied via targeted `rubocop -a`, not a blanket run: the wider codebase has ~1000 pre-existing offences (Style/GlobalVars, heredoc naming) that are out of scope here. Generated output is byte-identical (oracle: 53 passed on a clean build).
Running the generated-file comparison oracle (pytest) leaves utils/__pycache__ behind, which is easy to sweep into a commit by accident.
The comparison oracle gained backends/opencl/opencl_model.yaml and btx_cl_model.yaml, but the CI build step only asked for libOpenCL.la and opencl_profiling.tp. Neither pulls those two in -- btx_cl_model.yaml is a plain intermediate -- so the oracle hit FileNotFoundError on the base branch. Name them on the make line, as every other backend already does for its model yaml. Verified by reproducing the CI build locally: both files are absent before this change and present after, and identical between base and PR.
load_meta_parameters read content['meta_parameters'] and silently registered nothing when the key was absent, which is indistinguishable from a backend that genuinely has none. A typo in the key -- or the string-vs-symbol confusion -- therefore dropped every entry in the file without a word. Raise instead. A backend with no meta-parameters of its own already has a way to say so explicitly: `meta_parameters: []`, as cudart does. This check is what caught mpi_meta_parameters.yaml using the symbol key `:meta_parameters:`, which had left all of its entries dead since 9ae9297. That file was reconciled with the current code in f2ae743, so every backend builds.
TApplencourt
force-pushed
the
handle-uuid-hoist
branch
from
August 13, 2026 18:30
ffb3d74 to
44e9396
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.