https://gcc.gnu.org/g:622b68c6e14b9c289f64709ca772b02cb5855cb1
commit r16-7755-g622b68c6e14b9c289f64709ca772b02cb5855cb1 Author: Soumya AR <[email protected]> Date: Thu Feb 26 04:55:48 2026 +0000 aarch64: Fix NULL structures in base tunings being silently ignored by JSON parser In the JSON parser's parse_object_helper for const pointer members, we have a if (!member) return; guard at entry. This is because the function works by dereferencing the member pointer and making a non-const copy of the object it points to. The parser can then override this copy with new values and re-assign the pointer to the new object. This is problematic if member is NULL in the base tunings, but provided in the JSON file, which can happen with some combinations of -mcpu and -muser-provided-CPU. For example, with no -mcpu, if the generic tunings has issue_info set to nullptr (eg. generic_armv8_a), then even if the user does provide issue_info data in the JSON file, parse_object_helper will silently ignore it. The naive fix for this is to create a zero-initialized copy of the object if it is NULL, and then override it with the new values from the JSON file. However, this results in another problem: if the user provides only selective fields of the strucutre, the rest of the fields will be set to zero and potentially interfere with costing decisions. I think at that point the best we can do is emit a warning. With David Malcolm's improved JSON diagnostics, we can be specific about the problematic structure as well. ---- Since we potentially zero-initialize objects, I had to add a default constructor to objects that only had parameterized constructors. Bootstrapped and regtested on aarch64-linux-gnu, OK for trunk? Signed-off-by: Soumya AR <[email protected]> gcc/ChangeLog: * config/aarch64/aarch64-json-tunings-parser.cc (parse_object_helper): Zero-initialize objects that are NULL in the base tunings, if provided in JSON tunings. * config/aarch64/aarch64-protos.h (struct sve_vec_cost): Add default constructor. (struct aarch64_simd_vec_issue_info): Likewise. (struct aarch64_sve_vec_issue_info): Likewise. gcc/testsuite/ChangeLog: * gcc.target/aarch64/aarch64-json-tunings/nullptr-issue-info.c: New test. * gcc.target/aarch64/aarch64-json-tunings/nullptr-issue-info.json: New test input. Diff: --- gcc/config/aarch64/aarch64-json-tunings-parser.cc | 8 ++--- gcc/config/aarch64/aarch64-protos.h | 6 ++++ .../aarch64-json-tunings/nullptr-issue-info.c | 20 ++++++++++++ .../aarch64-json-tunings/nullptr-issue-info.json | 38 ++++++++++++++++++++++ 4 files changed, 68 insertions(+), 4 deletions(-) diff --git a/gcc/config/aarch64/aarch64-json-tunings-parser.cc b/gcc/config/aarch64/aarch64-json-tunings-parser.cc index 9b7bdf535c9b..195980347619 100644 --- a/gcc/config/aarch64/aarch64-json-tunings-parser.cc +++ b/gcc/config/aarch64/aarch64-json-tunings-parser.cc @@ -109,9 +109,6 @@ static std::enable_if_t<std::is_pointer<T>::value parse_object_helper (const json::object *field_obj, T &member, parse_func_type<T> parse_func) { - if (!member) - return; - /* Use static storage for the non-const copy. This works because tune_params does not have nested structures of the same type, but has room for errors if we end up having pointers to the @@ -125,7 +122,10 @@ parse_object_helper (const json::object *field_obj, T &member, } already_initialized = true; using NonConstType = std::remove_const_t<std::remove_pointer_t<T>>; - static NonConstType new_obj = *member; + if (!member) + warning (0, "JSON tuning overrides an unspecified structure in the base " + "tuning; fields not provided in JSON will default to 0"); + static NonConstType new_obj = member ? *member : NonConstType{}; parse_func (field_obj, new_obj); member = &new_obj; } diff --git a/gcc/config/aarch64/aarch64-protos.h b/gcc/config/aarch64/aarch64-protos.h index d1f2873f208b..3f359b0069d1 100644 --- a/gcc/config/aarch64/aarch64-protos.h +++ b/gcc/config/aarch64/aarch64-protos.h @@ -255,6 +255,8 @@ typedef struct simd_vec_cost advsimd_vec_cost; /* SVE-specific extensions to the information provided by simd_vec_cost. */ struct sve_vec_cost : simd_vec_cost { + sve_vec_cost () = default; + CONSTEXPR sve_vec_cost (const simd_vec_cost &base, unsigned int clast_cost, unsigned int fadda_f16_cost, @@ -365,6 +367,8 @@ using aarch64_scalar_vec_issue_info = aarch64_base_vec_issue_info; Advanced SIMD and SVE. */ struct aarch64_simd_vec_issue_info : aarch64_base_vec_issue_info { + aarch64_simd_vec_issue_info () = default; + CONSTEXPR aarch64_simd_vec_issue_info (aarch64_base_vec_issue_info base, unsigned int ld2_st2_general_ops, unsigned int ld3_st3_general_ops, @@ -393,6 +397,8 @@ using aarch64_advsimd_vec_issue_info = aarch64_simd_vec_issue_info; is a concept of "predicate operations". */ struct aarch64_sve_vec_issue_info : aarch64_simd_vec_issue_info { + aarch64_sve_vec_issue_info () = default; + CONSTEXPR aarch64_sve_vec_issue_info (aarch64_simd_vec_issue_info base, unsigned int pred_ops_per_cycle, diff --git a/gcc/testsuite/gcc.target/aarch64/aarch64-json-tunings/nullptr-issue-info.c b/gcc/testsuite/gcc.target/aarch64/aarch64-json-tunings/nullptr-issue-info.c new file mode 100644 index 000000000000..3984c63b39ef --- /dev/null +++ b/gcc/testsuite/gcc.target/aarch64/aarch64-json-tunings/nullptr-issue-info.c @@ -0,0 +1,20 @@ +/* Test that a structure provided in JSON is parsed even when the base tuning + model has the structure set to NULL. */ + +/* { dg-do compile } */ +/* { dg-additional-options "-muser-provided-CPU=${srcdir}/gcc.target/aarch64/aarch64-json-tunings/nullptr-issue-info.json -fdump-tuning-model=temp-nullptr-issue-info.json" } */ + +/* { dg-warning "JSON tuning overrides an unspecified structure in the base tuning; fields not provided in JSON will default to 0" "" { target *-*-* } 0 } */ +/* { dg-warning "JSON tuning file does not contain version information" "" { target *-*-* } 0 } */ + +/* { dg-final { scan-file "temp-nullptr-issue-info.json" "\"loads_stores_per_cycle\": 4" } } */ +/* { dg-final { scan-file "temp-nullptr-issue-info.json" "\"general_ops_per_cycle\": 8" } } */ + +/* { dg-final { scan-file "temp-nullptr-issue-info.json" "\"ld2_st2_general_ops\": 2" } } */ +/* { dg-final { scan-file "temp-nullptr-issue-info.json" "\"ld4_st4_general_ops\": 3" } } */ + +/* { dg-final { scan-file "temp-nullptr-issue-info.json" "\"pred_ops_per_cycle\": 2" } } */ +/* { dg-final { scan-file "temp-nullptr-issue-info.json" "\"while_pred_ops\": 1" } } */ +/* { dg-final { scan-file "temp-nullptr-issue-info.json" "\"gather_scatter_pair_general_ops\": 1" } } */ + +int main () {} diff --git a/gcc/testsuite/gcc.target/aarch64/aarch64-json-tunings/nullptr-issue-info.json b/gcc/testsuite/gcc.target/aarch64/aarch64-json-tunings/nullptr-issue-info.json new file mode 100644 index 000000000000..b6fefa87544c --- /dev/null +++ b/gcc/testsuite/gcc.target/aarch64/aarch64-json-tunings/nullptr-issue-info.json @@ -0,0 +1,38 @@ +{ + "tune_params": { + "vec_costs": { + "issue_info": { + "scalar": { + "loads_stores_per_cycle": 4, + "stores_per_cycle": 2, + "general_ops_per_cycle": 8, + "fp_simd_load_general_ops": 0, + "fp_simd_store_general_ops": 1 + }, + "advsimd": { + "loads_stores_per_cycle": 3, + "stores_per_cycle": 2, + "general_ops_per_cycle": 6, + "fp_simd_load_general_ops": 0, + "fp_simd_store_general_ops": 1, + "ld2_st2_general_ops": 2, + "ld3_st3_general_ops": 2, + "ld4_st4_general_ops": 3 + }, + "sve": { + "loads_stores_per_cycle": 3, + "stores_per_cycle": 2, + "general_ops_per_cycle": 6, + "fp_simd_load_general_ops": 0, + "fp_simd_store_general_ops": 1, + "pred_ops_per_cycle": 2, + "while_pred_ops": 1, + "int_cmp_pred_ops": 0, + "fp_cmp_pred_ops": 0, + "gather_scatter_pair_general_ops": 1, + "gather_scatter_pair_pred_ops": 1 + } + } + } + } +}
