Re: [PATCH v6 1/4] arc: Add ARCv2 XML target along with refactoring

Simon Marchi
2020-08-21T16:16:12Z

Hi Shahab,

The only thing code-wise that changed between v5 and v6 is looking for obsolete features names?

If so, that LGTM (I didn't re-read the whole patch, just checked that and it looks reasonable)
with these nits fixed:

> -static const char *const aux_minimal_register_names[] = {
> - "pc", "status32",
> +/* Obsolete feature names for backward compatibility. */
> +static const char *ARC_CORE_V1_OBSOLETE_FEATURE_NAME
> + = "org.gnu.gdb.arc.core.arcompact";
> +static const char *ARC_CORE_V2_OBSOLETE_FEATURE_NAME
> + = "org.gnu.gdb.arc.core.v2";
> +static const char *ARC_CORE_V2_REDUCED_OBSOLETE_FEATURE_NAME
> + = "org.gnu.gdb.arc.core-reduced.v2";
> +static const char *ARC_AUX_OBSOLETE_FEATURE_NAME
> + = "org.gnu.gdb.arc.aux-minimal";

Should probably be two spaces for indentation here.

> +/* Look for obsolete core feature names in TDESC_DATA. */
> +
> +static const struct tdesc_feature *
> +find_obsolete_core_names (const struct target_desc *tdesc)

The parameter name doesn't match the doc, TDESC_DATA -> TDESC.

> +{
> + const struct tdesc_feature *feat = nullptr;
> +
> + feat = tdesc_find_feature (tdesc, ARC_CORE_V1_OBSOLETE_FEATURE_NAME);
> +
> + if (feat == nullptr)
> + feat = tdesc_find_feature (tdesc, ARC_CORE_V2_OBSOLETE_FEATURE_NAME);
> +
> + if (feat == nullptr)
> + feat = tdesc_find_feature
> + (tdesc, ARC_CORE_V2_REDUCED_OBSOLETE_FEATURE_NAME);
> +
> + return feat;
> +}
> +
> +/* Look for obsolete aux feature names in TDESC_DATA. */
> +
> +static const struct tdesc_feature *
> +find_obsolete_aux_names (const struct target_desc *tdesc)

Same here.

Simon