On Tue, Aug 7, 2012 at 8:38 PM, Lawrence Crowl <cr...@google.com> wrote:
> On 8/7/12, Richard Guenther <richard.guent...@gmail.com> wrote:
>> On Tue, Aug 7, 2012 at 2:35 AM, Lawrence Crowl <cr...@google.com> wrote:
>> > Convert double_int from a struct with function into a class with
>> > operators and methods.
>> >
>> > This patch adds the methods and operators. In general functions of
>> > the form "double_int_whatever" become member functions "whatever" or,
>> > when possible, operators.
>> >
>> > Every attempt has been made to preserve the existing algorithms, even
>> > at the expense of some optimization opportunities. Some examples:
>> >
>> > The ext operation takes a value and returns a value. However, that
>> > return value is usually assigned to the original variable. An
>> > operation that modified a variable would be more efficient.
>>
>> That's not always the case though and I think the interface should be
>> consistent with existing behavior to avoid errors creeping in during the
>> transition.
>
> We should probably think about naming conventions for mutating operations,
> as I expect we will want them eventually.
Right. In the end I would prefer explicit constructors.
>> > In some cases, an outer sign-specific function calls an inner
>> > function with the sign as a parameter, which then decides which
>> > implementation to do. Decisions should not be artificially
>> > introduced, and the implementation of each case should be exposed as
>> > a separate routine.
>> >
>> > The existing operations are implemented in terms of the new
>> > operations, which necessarily adds a layer between the new code and
>> > the existing users. Once all files have migrated, this layer will
>> > be removed.
>> >
>> > There are several existing operations implemented in terms of even
>> > older legacy operations. This extra layer has not been removed.
>> >
>> > On occasion though, parameterized functions are often called
>> > with a constant argments. To support static statement of intent,
>> > and potentially faster code in the future, there are several new
>> > unparameterized member functions. Some examples:
>> >
>> > Four routines now encode both 'arithmetic or logical' and 'right or
>> > left' shift as part of the funciton name.
>> >
>> > Four routines now encode both 'signed or unsigned' and 'less than or
>> > greater than' as part of the function name.
>>
>> For most parts overloads that take an (unsigned) HOST_WIDE_INT argument
>> would be nice, as well as the ability to say dbl + 1.
>
> Hm. There seems to be significant opinion that there should not be any
> implicit conversions. I am okay with operations as above, but would like
> to hear the opinions of others.
Well, I'd simply add
double_int operator+(HOST_WIDE_INT);
double_int operator+(unsigned HOST_WIDE_INT);
and be done with it ;) Yes, a tad bit more inconvenient on the implementation
side compared to a non-explicit constructor from HOST_WIDE_INT or a
conversion operator ... but adhering to the rule that we do _not_ want such
automatic conversions (well, yet, for the start).
>> > -typedef struct
>> > +typedef struct double_int
>> > {
>> > +public:
>> > + /* Normally, we would define constructors to create instances.
>> > + Two things prevent us from doing so.
>> > + First, defining a constructor makes the class non-POD in C++03,
>> > + and we certainly want double_int to be a POD.
>> > + Second, the GCC conding conventions prefer explicit conversion,
>> > + and explicit conversion operators are not available until C++11.
>> > */
>> > +
>> > + static double_int make (unsigned HOST_WIDE_INT cst);
>> > + static double_int make (HOST_WIDE_INT cst);
>> > + static double_int make (unsigned int cst);
>> > + static double_int make (int cst);
>>
>> Did we somehow decide to not allow constructors? It's odd to convert to
>> C++ and end up with static member functions resembling them ...
>
> Constructors are allowed, but PODs are often passed more efficiently.
> That property seemed particularly important for double_int.
True - I forgot about this issue. Same for the return values. I suppose we
need to inspect code quality before going this route.
>> Also I believe the conversion above introduces possible migration errors.
>> Think of a previous
>>
>> HOST_WIDE_INT a;
>> double_int d = uhwi_to_double_int (a);
>>
>> if you write that now as
>>
>> HOST_WIDE_INT a;
>> double_int d = double_int::make (a);
>>
>> you get the effect of shwi_to_double_int. Oops.
>
> Hm. I think the code was more likely to be wrong originally,
> but I take your point.
>
>> So as an intermediate
>> I'd like you _not_ to introduce the make () overloads.
>
> Okay.
>
>> Btw, if HOST_WIDE_INT == int the above won't even compile.
>
> Is that true of any host? I am not aware of any. Anyway, it is
> moot if we remove the overloads.
Right. I'd simply rely on int / unsigned int promotion to HOST_WIDE_INT
or unsigned HOST_WIDE_INT and only provide overloads for HOST_WIDE_INT
kinds anyway.
Richard.
> --
> Lawrence Crowl
