Bernd Schmidt <bschm...@redhat.com> writes:
> On 10/05/2015 05:22 PM, Richard Sandiford wrote:
>> Bernd Schmidt <bschm...@redhat.com> writes:
>>> On 10/05/2015 04:47 PM, Richard Sandiford wrote:
>>>> @@ -9536,7 +9520,7 @@ fold_builtin_classify (location_t loc, tree fndecl, 
>>>> tree arg, int builtin_index)
>>>>            {
>>>>              r = TREE_REAL_CST (arg);
>>>>              if (real_isinf (&r))
>>>> -      return real_compare (GT_EXPR, &r, &dconst0)
>>>> +      return real_compare (GT_EXPR, &r, &dconst<0> ())
>>>>                       ? integer_one_node : integer_minus_one_node;
>>>>              else
>>>>                return integer_zero_node;
>>>
>>> So... are the templates magic enough not to make us create a new
>>> temporary every time this is used?
>>
>> Yeah, the static variables become comdat objects keyed off the full name
>> (dconst<...>::value).  They're shared between calls and between TUs.
>
> Hmm. And since you're returning a reference, taking the address works. 
> The whole thing is subtle enough that it deserves a comment. Since this 
> kind of thing is something I don't like about C++ (simple-looking code 
> expanding into non-obvious behaviour) I'm not going to ack this patch, 
> but if someone else wants to, that's fine.
>
> I do believe you still have some code growth since the inline dconst 
> function always expands code that will initialize the constant. IMO 
> that's not desirable.

I don't disagree.  I find dconst0 much easier to read than dconst<0> ().
In some ways I was posting the patch to show how bad it could be :-)

If we're prepared to pay the cost of unconditional load-time
initialisation, we could have:

template <int N>
struct dconst_value
{
  dconst_value () { ...set up value ...; }
  REAL_VALUE_TYPE value;
};

template <int N>
const REAL_VALUE_TYPE &
dconst<T> (void)
{
  static dconst_value<N> x;
  return x.value;
}

But this feels like using templates just to prove we know how they work,
rather than because they're the right tool for the job.

If my original patch isn't acceptable, another old-school way of doing
it would be to have a .def file of all the constants that we want.
They could then all be global variables, rather than having some that
are and some that aren't.  They could be initialised at the same point
that dconst0 etc. are initialised now.

E.g. we could have dconst1_3 for 1/3, dconst1_9 for 1/9, etc.  There's
a slight problem that those names are already used for local, rounded,
versions of the constants in some places, but that's easy to fix.

Thanks,
Richard

Reply via email to