This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Make BITMAP_WORD more easily configurable


On 8/1/19 2:49 AM, Richard Biener wrote:
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)

With the overload approach one would need to be provided for each
integer type starting with int, both signed and unsigned.  With
C++ templates (SFINAE and type traits) there may be ways to
reduce the number of definitions at the cost of the complexity
of the implementation (while still keeping the interface simple).

The biggest drawback of "mangling" the type name in the names of
functions is that users have to know what type they're dealing
with, and remember the mangling scheme (is it foo_ull or foo_llu
or foo_ullong or something else?).  Even in C with typedefs that's
an error-prone PITA.  A mistake can cause bugs due to slicing.
In C++ it makes the functions unsuitable for use in templates.
In both languages it makes it difficult to change the data types
used in data structures.  Going from long to int like in your
experiment means changing the code that works on the data.  C
solves it by "parameterizing" everything on macros.  The C++
approach is templates.  Mixing the two doesn't work very well.

Martin


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.



Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]