Re: [middle-end] Add machine_mode to address_cost target hook

[Richard Sandiford <rdsandiford at googlemail dot com>]

Oleg Endo <oleg.endo@t-online.de> writes:
> On Sat, 2012-09-01 at 10:10 +0100, Richard Sandiford wrote:
>
>> Thanks for doing this.  We should perhaps add the address space too,
>> but if you don't feel like redoing the whole patch, that can wait until
>> someone wants it.
>
> I just had a look at the address space thing...
> There are already target hook overloads with address space parameters,
> like legitimate_address_p and legitimize_address.  Paolo suggested to do
> something similar a while ago:
> http://gcc.gnu.org/ml/gcc-patches/2009-06/msg01191.html
>
> .. but somehow this never made it into mainline?!
>
> Maybe it would be better to ...
> a) Add an 'address_cost' overload to the existing onces.
>    (at least it would be consistent...)
> b) Remove the overloads altogether and add the address space arg to the
> original funcs.
>
> Personally, I'd probably favor b).  Some targets (e.g. rl78) already
> override the address space overloads and just ignore the address space
> arg.  Either way, maybe it's better to do it in a separate patch.  This
> looks a bit like a pandora's box :)

Well, I certainly wasn't suggesting you had to do anything with
overloaded hooks (which I agree are unfortunate).  It's just that
rtlanal.c:address_cost already gets an address space argument, so if
we're going through all the ports adding the mode argument to the hook,
it seemed a pity not to add the address space argument at the same time.

I don't see anything wrong with adding address space arguments to hooks
outside the addr_space substructure.

But like I say, I won't insist.

>> >  /* If the target doesn't override, compute the cost as with arithmetic.  */
>> >  
>> >  int
>> > -default_address_cost (rtx x, bool speed)
>> > +default_address_cost (rtx x, enum machine_mode mode, bool speed)
>> >  {
>> >    return rtx_cost (x, MEM, 0, speed);
>> >  }
>> 
>> Remove the "mode" parameter name because it's unused.  I think this
>> would trigger a warning otherwise.
>
> Removing won't do here, because of:
>
> Index: gcc/target.def
> ===================================================================
> --- gcc/target.def	(revision 190840)
> +++ gcc/target.def	(working copy)
> @@ -1758,7 +1758,7 @@
>  DEFHOOK
>  (address_cost,
>   "",
> - int, (rtx address, bool speed),
> + int, (rtx address, enum machine_mode mode, bool speed),
>   default_address_cost)
>
>
> Instead I've added ATTRIBUTE_UNUSED to avoid the warning.

Note that I said remove the "mode" parameter _name_, not the parameter
itself :-)  We're C++ now, so leaving out the name is better than
supplying an unused one and then adding ATTRIBUTE_UNUSED to it.

Richard