Hi Mike,

On Fri, Jun 28, 2019 at 02:50:33PM -0400, Michael Meissner wrote:
> --- gcc/config/rs6000/rs6000-logue.c  (revision 272714)
> +++ gcc/config/rs6000/rs6000-logue.c  (working copy)
> @@ -1406,23 +1406,13 @@ uses_TOC (void)
>  }
>  #endif
>  
> +/* Create a TOC style reference for a symbol.  */
>  rtx
>  create_TOC_reference (rtx symbol, rtx largetoc_reg)

Does this really belong in this file?  It doesn't do anythin *logue, and
it isn't called from anywhere in here.

> +/* Create either a TOC reference to a locally defined item or a pc-relative
> +   reference, depending on the ABI.  */
> +rtx
> +create_data_reference (rtx symbol, rtx largetoc_reg)

Same here.

What is largetoc_reg?  The function comment should say.  It also is only
relevant for create_TOC_reference (where such a comment is also missing),
so could you factor this better please?

Probably a create_data_reference with only one argument?  Which calls
create_TOC_reference with a NULL second arg.  It looks like your
proposed create_data_reference will not do the right thing if called
with a non-null second arg if pcrel.  Perhaps that cannot happen, but
make that clear then?  Just an assert will do, bigger cleanups are
better of course.


Segher

Reply via email to