Re: [PATCH] Make BITMAP_WORD more easily configurable

[Richard Biener <rguenther at suse dot de>]

On Wed, 31 Jul 2019, Martin Sebor wrote:

> On 7/31/19 4:00 AM, Richard Biener wrote:
> > On Tue, 30 Jul 2019, Martin Sebor wrote:
> > 
> > > On 7/30/19 9:19 AM, Jakub Jelinek wrote:
> > > > On Tue, Jul 30, 2019 at 09:16:45AM -0600, Martin Sebor wrote:
> > > > > Say something like this POC:
> > > > 
> > > > We have those already, see ctz_hwi, clz_hwi etc.
> > > > The thing is that BITMAP_WORD isn't always the same type as unsigned
> > > > HOST_WIDE_INT, and so we need ones with the BITMAP_WORD type.
> > > 
> > > That sounds like a problem overloading would solve to the benefit
> > > of all clients.  I.e., overload count_trailing_zeros (or whatever
> > > other name) to take integer arguments of all widths (signed and
> > > unsigned) and call the appropriate built-in from each.  That way
> > > we just have to remember one function name and know that it will
> > > be the right choice regardless of the type of the argument.
> > 
> > The problem with clz at least is that you have to make sure
> > to only allow overloads to exactly match the type of the
> > operand since semantics change once you do any promotion.
> > For popcount and ctz that's only an issue for signed arguments,
> > but still.
> > 
> > And I don't see any optimization of (uint){clzl,popcountl}((ulong)uintvar)
> > -> {clz,popcount} (uintvar)
> > 
> > unsigned int foo (unsigned int x)
> > {
> >    unsigned long y = x;
> >    unsigned long z = __builtin_popcountl (y);
> >    return z;
> > }
> > 
> > calls __popcountdi2 for me.
> > 
> > So if you dislike the macros then we could add
> > {popcount,clz,ctz}_bitmap_word () functions.
> > 
> > Still having the plumbing in once place looks like an improvement
> > to me - not sure if your comments were an objection to the original
> > patch?
> 
> Not an objection, just as an encouragement to consider going
> the wrapper route you mentioned.  I think that would make
> for a more broadly applicable improvement with a more user
> friendly interface than what's available today.

Overload resolution is a bit weird given it seems to be applied
after promotion and given it is not matching "identical" types
like long long and long as soon as I have two overloads, one
for int and one for long.

So I'd rather have the type explicit in the name of the function
since semantics depend on the actual chosen computation type.

Otherwise we'd have to provide overloads for all standard integer
types and simply not implement some (but what's the point then...).
For that using templates and specialization is probably a better
match then (same issue with long vs. long long but that could
be solved with more template magic).

template <typename T>
int
gcc_popcount (T) { typename T::not implemented; }

template <>
int
gcc_popcount (HOST_WIDE_INT x) { return popcount_whi (x); }

etc.

?  (or xpopcount to mimic libiberty style naming)

As said it would still say not implemented for 'long long' or
'long' depending on how HOST_WIDE_INT is defined.  More C++-fu
can solve this though.

Richard.