Hi Mike,
On Thu, Jun 27, 2019 at 08:12:35PM -0400, Michael Meissner wrote:
> + return (TARGET_CMODEL == CMODEL_MEDIUM && SYMBOL_REF_P (op)
> + && SYMBOL_REF_LOCAL_P (op));
Please break the line before that first && as well?
> +(define_predicate "pcrel_external_address"
> + (match_code "symbol_ref,const")
> +{
> + if (!TARGET_PCREL || TARGET_CMODEL != CMODEL_MEDIUM)
> + return false;
if (!(TARGET_PCREL && TARGET_CMODEL == CMODEL_MEDIUM))
Should there be a helper function for this? PCREL is only supported
for medium model -- abstracting that makes both the current code easier
to read, and if we ever want to support other models, that will be
simpler to do as well.
> + /* Discard any CONST's. */
> + if (GET_CODE (op) == CONST)
> + op = XEXP (op, 0);
That comment says nothing the code already tells. It's a common thing
to do anyway; just don't comment on it?
> +;; Return 1 if op is a memory operand to an external variable when we
> +;; support pc-relative addressing and the PCREL_OPT relocation to
> +;; optimize references to it.
> +(define_predicate "pcrel_external_mem_operand"
> + (match_code "mem")
> +{
> + return pcrel_external_address (XEXP (op, 0), Pmode);
> +})
Is that comment correct? pcrel_external_address does nothing with
PCREL_OPT? Or _its_ comment doesn't say, at least.
Okay for trunk with those trivialities resolved. Thanks!
Segher
