Andrew Carlotti <andrew.carlo...@arm.com> writes:
> There was an assumption in some places that the aarch64_fmv_feature_data
> array contained FEAT_MAX elements. While this assumption held up till
> now, it is safer and more flexible to use the array size directly.
>
> gcc/ChangeLog:
>
> * config/aarch64/aarch64.cc (compare_feature_masks):
> Use ARRAY_SIZE to determine iteration bounds.
> (aarch64_mangle_decl_assembler_name): Ditto.
>
>
> diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc
> index
> 1ea84c8bd7386e399f6ffa3a5e36408cf8831fc6..5de842fcc212c78beba1fa99639e79562d718579
> 100644
> --- a/gcc/config/aarch64/aarch64.cc
> +++ b/gcc/config/aarch64/aarch64.cc
> @@ -19899,7 +19899,8 @@ compare_feature_masks (aarch64_fmv_feature_mask mask1,
> auto diff_mask = mask1 ^ mask2;
> if (diff_mask == 0ULL)
> return 0;
> - for (int i = FEAT_MAX - 1; i > 0; i--)
> + static const int num_features = ARRAY_SIZE (aarch64_fmv_feature_data);
There doesn't seem any need for this to be static (or const). Same for
the second hunk.
> + for (int i = num_features - 1; i > 0; i--)
Pre-existing, but is > 0 rather than >= 0 deliberate? Shouldn't we look
at index 0 as well?
LGTM otherwise.
Thanks,
Richard
> {
> auto bit_mask = aarch64_fmv_feature_data[i].feature_mask;
> if (diff_mask & bit_mask)
> @@ -19982,7 +19983,8 @@ aarch64_mangle_decl_assembler_name (tree decl, tree
> id)
>
> name += "._";
>
> - for (int i = 0; i < FEAT_MAX; i++)
> + static const int num_features = ARRAY_SIZE (aarch64_fmv_feature_data);
> + for (int i = 0; i < num_features; i++)
> {
> if (feature_mask & aarch64_fmv_feature_data[i].feature_mask)
> {
