Re: [cxx-conversion] Make double_int a class with methods and operators. (issue6443093)

2012-08-09 Thread Mike Stump
On Aug 9, 2012, at 5:00 PM, Michael Matz wrote:
> On Thu, 9 Aug 2012, Mike Stump wrote:
> 
>> On Aug 9, 2012, at 8:19 AM, Michael Matz wrote:
>>> Hmm.  And maintaining a cache is faster than 
>>> passing/returning/manipulating two registers?
>> 
>> For the most part, we merely mirror existing code, check out 
>> lookup_const_double and immed_double_const.
> 
> No, I won't without patches on this list.

Ah, we are discussing the code in the gcc tree currently.  You _can_ comment on 
it, if you like to.  I was only pointing out that this choice we didn't make 
nor deviate from the code in the top of the tree.  If you think it is wrong to 
cache it, then talking about the code in the top of the tree is the right place 
to discuss it.  Though, you don't have to if you don't want to.

> You keep repeating bragging

Such hostility.  Why?  I don't get it.  I _asked_ about when the cxx branch was 
going to land, I stated that I liked non-mutating interfaces, I gave a heads up 
that we have a wide-int class to replace double-int for ints.  I _only_ gave a 
heads up because of the submitted change to the cxx branch conflicts on a 
larger than expected scale with the wide-int change.  I think giving a heads up 
before the conflict happens is good citizenship.

> I mean, preparing the audience for an upcoming _suggested_ change in data 
> structure of course is fine.  But argueing as if the change happenend 
> already, and what's more concerning, as if the change was even already 
> suggested and agreed upon even though that's not the case, is just bad 
> style.

So, let me get this straight, alerting people that I have a patch that 
conflicts with another posted patch is, bad style?  Odd.  I saw it listed on 
page 10 of the etiquette guide, maybe you could update the guide for us.

> I would suggest to stay conservative about whatever you have (except if 
> it's momentarily materializing), and _especially don't argue against or 
> for or not against or for whatever improvement is suggested

Ah, that's a misunderstanding on your part.  I was not arguing for, or against 
the double_int changes.  In fact, I'm very supportive of those changes and the 
entire cxx branch, not that you'd know that, as I think all of the changes are 
a slam dunk and don't need any support from me.  The :-( in the email that you 
read, was just a comment that someone is going to have to resolve conflicts.  
Now that we know the timing of the cxx branch landing, we expect, we'll handle 
the conflicts on the wide-int side.  If the timing was different, we'd land the 
wide-int change first, then the :-( in the heads up comment would be read more 
as, we're sorry, but we've just scrambled the tree on you, so sorry.

Let me be perfectly clear, I support the double_int changes and the entire 
cxx-conversion branch.  No work I may or may not have matters or should be 
considered in reviewing any patches.  I'm a firm believer in the first in, wins 
method of resolving conflicts.  Sorry if anyone thought I was objecting in 
anyway to the double_int work.

> Nobody has seen it yet,

Actually, that's not true; but, it doesn't matter any.

> so you can't expect to get any feedback on it.

I don't recall asking for feedback on it.  The feedback I requested that you 
quote above, was feedback on the code in the top of the tree.


Re: [cxx-conversion] Make double_int a class with methods and operators. (issue6443093)

2012-08-09 Thread Michael Matz
Hi,

On Thu, 9 Aug 2012, Mike Stump wrote:

> On Aug 9, 2012, at 8:19 AM, Michael Matz wrote:
> > Hmm.  And maintaining a cache is faster than 
> > passing/returning/manipulating two registers?
> 
> For the most part, we merely mirror existing code, check out 
> lookup_const_double and immed_double_const.

No, I won't without patches on this list.  You keep repeating bragging 
about wide_int during the last two weeks, without offering anything 
concrete about it whatsoever.  You'll understand that I (or anybody else) 
can't usefully discuss with you any merits or demerits of the 
implementation you chose.  (can I btw. complain about the retainment of 
underscores?  If it's a base data type, then why not wideint?  Make that a 
testament for the "quality" of feedback you'll get with the information 
given)

I mean, preparing the audience for an upcoming _suggested_ change in data 
structure of course is fine.  But argueing as if the change happenend 
already, and what's more concerning, as if the change was even already 
suggested and agreed upon even though that's not the case, is just bad 
style.

I would suggest to stay conservative about whatever you have (except if 
it's momentarily materializing), and _especially don't argue against or 
for or not against or for whatever improvement is suggested on the grounds 
that you have a better, as of yet secret but surely taking-over-the-world 
very-soon-now implementation of datastructure X_.  Nobody has seen it yet, 
so you can't expect to get any feedback on it.  Certainly that's the thing 
you need to get it into the code base.

> If the existing code is wrong, love to have someone fix it.  :-)  Also, 
> bear in mind, on a port with with OImode math for example, on a 32-bit 
> host, it would be 8 registers...

Nice try.  But what problem do _you_ want to solve?  For instance why 
should a port with OImode for example be interesting to the FSF?  I hope 
you recognize this as half-rhethorical question, but still, how exactly 
will wide_int help for the goal (which remains to be shown as useful), how 
is it implemented?, why isn't it worse than crap on sensible (i.e. 64bit) 
hosts, and why should everybody not interested in such target pay the 
price, or why isn't there a price to pay for non-OI-targets?

I'm actually more intersted in comments to the first part, but still, 
comments on OI appreciated.


Ciao,
Michael.


Re: [cxx-conversion] Make double_int a class with methods and operators. (issue6443093)

2012-08-09 Thread Mike Stump
On Aug 9, 2012, at 8:19 AM, Michael Matz wrote:
> Hmm.  And maintaining a cache is faster than 
> passing/returning/manipulating two registers?

For the most part, we merely mirror existing code, check out 
lookup_const_double and immed_double_const.  If the existing code is wrong, 
love to have someone fix it.  :-)  Also, bear in mind, on a port with with 
OImode math for example, on a 32-bit host, it would be 8 registers...


Re: [cxx-conversion] Make double_int a class with methods and operators. (issue6443093)

2012-08-09 Thread Michael Matz
Hi,

On Thu, 9 Aug 2012, Mike Stump wrote:

> > Ah.  For simple objects like double_int I prefer to have either all 
> > ops mutating or all ops non-mutating.
> 
> wide_int, which replaces double_int for int types, is always 
> non-mutating, by value interface.  In C++, it will be const & input 
> parameters, to avoid the copies and retain the performance. We maintain 
> a cache under it, and reuse out of it for the long lived objects, for 
> short lived, we just allocate the on the stack as needed.

Hmm.  And maintaining a cache is faster than 
passing/returning/manipulating two registers?


Ciao,
Michael.


Re: [cxx-conversion] Make double_int a class with methods and operators. (issue6443093)

2012-08-09 Thread Mike Stump
On Aug 9, 2012, at 1:22 AM, Richard Guenther wrote:
> Ah.  For simple objects like double_int I prefer to have either all ops 
> mutating
> or all ops non-mutating.

wide_int, which replaces double_int for int types, is always non-mutating, by 
value interface.  In C++, it will be const & input parameters, to avoid the 
copies and retain the performance. We maintain a cache under it, and reuse out 
of it for the long lived objects, for short lived, we just allocate the on the 
stack as needed.


Re: [cxx-conversion] Make double_int a class with methods and operators. (issue6443093)

2012-08-09 Thread Gabriel Dos Reis
On Thu, Aug 9, 2012 at 3:22 AM, Richard Guenther
 wrote:
> On Thu, Aug 9, 2012 at 12:25 AM, Lawrence Crowl  wrote:
>> On 8/8/12, Richard Guenther  wrote:
>>> On Aug 7, 2012 Lawrence Crowl  wrote:
>>> > 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.
>>
>> I don't think we're thinking about the same thing.
>>
>> I'm talking about member functions like mystring.append ("foo").
>> The += operator is mutating as well.
>>
>> Constructors do not mutate, they create.
>
> Ah.  For simple objects like double_int I prefer to have either all ops 
> mutating
> or all ops non-mutating.

Hmm, isn't that a bit extreme?  I mean that does not hold for simple
types that int
or double, etc.

-- Gaby


Re: [cxx-conversion] Make double_int a class with methods and operators. (issue6443093)

2012-08-09 Thread Richard Guenther
On Thu, Aug 9, 2012 at 12:25 AM, Lawrence Crowl  wrote:
> On 8/8/12, Richard Guenther  wrote:
>> On Aug 7, 2012 Lawrence Crowl  wrote:
>> > 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.
>
> I don't think we're thinking about the same thing.
>
> I'm talking about member functions like mystring.append ("foo").
> The += operator is mutating as well.
>
> Constructors do not mutate, they create.

Ah.  For simple objects like double_int I prefer to have either all ops mutating
or all ops non-mutating.

Richard.

> --
> Lawrence Crowl


Re: [cxx-conversion] Make double_int a class with methods and operators. (issue6443093)

2012-08-09 Thread Richard Guenther
On Thu, Aug 9, 2012 at 12:17 AM, Lawrence Crowl  wrote:
> On 8/7/12, Mike Stump  wrote:
>> On Aug 7, 2012, at 11:38 AM, Lawrence Crowl wrote:
>> > 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.
>>
>> If there is an agreed upon and expected semantic, having them are useful.
>> In the wide-int world, which replaces double_int, I think there is an
>> agreeable semantic and I think it is useful, so, I think we should plan on
>> having them, though, I'd be fine with punting their implementation until
>> such time as someone needs it.  If no one every needs the routine, I don't
>> see the harm in not implementing it.
>
> At present, there are no functions equivalent to (double_int + int), so
> there can be no expressions that need this overload.  I have no objection
> to adding such an overload, but if there are no objections, I would rather
> do it as a separate patch.

Sure.  It's just one of the possibilities to clean up existing code.

Richard.

> --
> Lawrence Crowl


Re: [cxx-conversion] Make double_int a class with methods and operators. (issue6443093)

2012-08-08 Thread Lawrence Crowl
On 8/8/12, Miles Bader  wrote:
> Mike Stump  writes:
> > > Constructors are allowed, but PODs are often passed more
> > > efficiently.  That property seemed particularly important
> > > for double_int.
> >
> > Show us the difference in timing.  Show us the generated code.
> > I can't imagine that it could ever matter.
>
> I'm also curious about that statement...  PODs don't really
> seem to offer much advantage with modern compilers, except in a
> few very specific cases (of which this doesn't seem to be one),
> e.g. in unions.

Which brings up a critical point that double_int is used in trees,
which are in a union, and so double_int must be a POD until trees
are no longer unions.

-- 
Lawrence Crowl


Re: [cxx-conversion] Make double_int a class with methods and operators. (issue6443093)

2012-08-08 Thread Lawrence Crowl
On 8/8/12, Gabriel Dos Reis  wrote:
> On Aug 8, 2012 Miles Bader  wrote:
> > Richard Guenther  writes:
> > > > > > Constructors are allowed, but PODs are often passed
> > > > > > more efficiently.  That property seemed particularly
> > > > > > important for double_int.
> > > > >
> > > > > Show us the difference in timing.  Show us the generated
> > > > > code.  I can't imagine that it could ever matter.
> > > >
> > > > I'm also curious about that statement...  PODs don't really
> > > > seem to offer much advantage with modern compilers, except
> > > > in a few very specific cases (of which this doesn't seem
> > > > to be one), e.g. in unions.
> > >
> > > They make a difference for the by-value passing ABI.
> > > double-ints can be passed in two registers on most platforms.
> >
> > Sure, but that doesn't seem to depend on PODness -- non-PODs
> > can be passed in two registers as well, AFAICS...
> >
> > E.g., in the following:
> >
> > typedef long valtype;
> >
> > struct X { valtype x, y; };
> > struct Y { valtype x, y;
> > >  Y (valtype a, valtype b) : x (a), y (b) { }
> > };
> >
> > extern void fx (X x);
> > void test_x () {X x = { 1, 2 }; fx (x); }
> >
> > extern void fy (Y y);
> > void test_y () {Y y (1, 2); fy (y); }
> >
> > test_x and test_y use exactly the same calling sequence (and
> > contain exactly the same assembly code)...  [on x86-64]
>
> It is not PODness in the standard sense that matters.  It is
> podness from the ABI perspecitve: Y does not have a user-defined
> copy-constructor nor a desctructor; it is not a polymorphic type,
> so it is OK to pass in registers just like X.

Well, more specifically, the Itanium ABI says if the copy constructor
and the destructor are trivial, one passes the class in registers.
Other ABIs can make other choices.  Older ABIs often pass all
structs and classes indirectly.

The adding in inline versions of various special members into
the following code results in the following worst-case changes.

x86 x86_64
instructions:   48 -> 69 = 144% 38 -> 53 = 139%
stack operations:6 -> 10 = 167%  2 ->  6 = 300%
memory operations:  28 -> 38 = 136% 22 -> 28 = 127%

These numbers are not huge, particularly in context, but worth
considering.

In any event, the point is moot.  To implement the exact semantics of
current expressions using shwi_to_double_int and uhwi_to_double_int,
we cannot use constructors anyway.  So, there remains no immediate
reason to use any constructors.


struct type
{
#ifdef DEFAULT
  type ();
#endif
#ifdef INITIAL
  type (int);
#endif
#ifdef COPYCON
  type (const type &from)
: field1 (from.field1),
  field2 (from.field2)
{
}
#endif
#ifdef DESTRUCT
  ~type ()
{
field1 = 0;
field2 = 0;
}
#endif
  type method (type);
  long int field1;
  long int field2;
};

extern type global;

void callee (type arg);

void function_caller ()
{
  type local (global);
  callee (local);
  callee (global);
}

void method_caller (type arg1, type arg2)
{
  type var1 (global);
  type var2 = arg2;
  arg1.method (arg2);
  var1.method (var2);
}

-- 
Lawrence Crowl


Re: [cxx-conversion] Make double_int a class with methods and operators. (issue6443093)

2012-08-08 Thread Lawrence Crowl
On 8/8/12, Richard Guenther  wrote:
> On Aug 8, 2012 Marc Glisse  wrote:
> > On Wed, 8 Aug 2012, Richard Guenther wrote:
> > > > > > +  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);
> >
> > [...]
> >
> > > > > 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.
> >
> > Sadly, that doesn't work with the current C++ rules (there is
> > a proposal to change that for C++1y):
> >
> > void f(long);
> > void f(unsigned long);
> > void g(int x){f(x);}
> >
> > e.cc: In function ‘void g(int)’:
> > e.cc:3:18: error: call of overloaded ‘f(int&)’ is ambiguous
> > e.cc:3:18: note: candidates are:
> > e.cc:1:6: note: void f(long int)
> > e.cc:2:6: note: void f(long unsigned int)
>
> Ick ... I forgot that integer promotions do not apply for int
> -> long.  So even f(1) would be ambiguous, right?  So I suppose
> we have to bite the bullet and add overloads for all (unsigned)
> integer types from int to HOST_WIDE_INT (if HOST_WIDE_INT is
> long long then we have to add overloads for int, unsigned int,
> long and unsigned long).
>
> Or use template magic to force promotion to HOST_WIDE_INT for
> integer types smaller than HOST_WIDE_INT...

I did not get to the structure I had accidentally.  However, with
the prior suggestion to preserve exact semantics to existing calls,
it is all moot because we cannot have overloading.

We can test whether the code is making double_ints with cross-sign
ints by adding undefined overloads.  I think we should do that and
make all such crossings explicit.  However, I want to wait for a
separate patch.

-- 
Lawrence Crowl


Re: [cxx-conversion] Make double_int a class with methods and operators. (issue6443093)

2012-08-08 Thread Lawrence Crowl
On 8/8/12, Richard Guenther  wrote:
> On Aug 7, 2012 Lawrence Crowl  wrote:
> > On 8/7/12, Richard Guenther  wrote:
> > > 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).

Ah, we have a different definition of implicit conversion.  In my
mind, the above overloads do an implicit conversion.  The issue
is that the calling code does not make the conversion obvious.
Note the difference in

   a = b + i;
   a = b + double_int(i);

Using overloads as above is generally safer than using an implicit
converting constructor or an implicit conversion operator.  If folks
are really only objecting to the implementation technique, we have
a somewhat different outcome.

-- 
Lawrence Crowl


Re: [cxx-conversion] Make double_int a class with methods and operators. (issue6443093)

2012-08-08 Thread Lawrence Crowl
On 8/8/12, Richard Guenther  wrote:
> On Aug 7, 2012 Lawrence Crowl  wrote:
> > 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.

I don't think we're thinking about the same thing.

I'm talking about member functions like mystring.append ("foo").
The += operator is mutating as well.

Constructors do not mutate, they create.

-- 
Lawrence Crowl


Re: [cxx-conversion] Make double_int a class with methods and operators. (issue6443093)

2012-08-08 Thread Lawrence Crowl
On 8/7/12, Mike Stump  wrote:
> On Aug 7, 2012, at 11:38 AM, Lawrence Crowl wrote:
> > 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.
>
> If there is an agreed upon and expected semantic, having them are useful.
> In the wide-int world, which replaces double_int, I think there is an
> agreeable semantic and I think it is useful, so, I think we should plan on
> having them, though, I'd be fine with punting their implementation until
> such time as someone needs it.  If no one every needs the routine, I don't
> see the harm in not implementing it.

At present, there are no functions equivalent to (double_int + int), so
there can be no expressions that need this overload.  I have no objection
to adding such an overload, but if there are no objections, I would rather
do it as a separate patch.

-- 
Lawrence Crowl


Re: [cxx-conversion] Make double_int a class with methods and operators. (issue6443093)

2012-08-08 Thread Lawrence Crowl
On 8/7/12, Richard Henderson  wrote:
> On 08/06/2012 05:35 PM, Lawrence Crowl wrote:
> > +inline double_int &
> > +double_int::operator ++ ()
> > +{
> > +  *this + double_int_one;
> > +  return *this;
> > +}
> > +
> > +inline double_int &
> > +double_int::operator -- ()
> > +{
> > +  *this - double_int_one;
> > +  return *this;
> > +}
>
> Surely unused results there?

Typically yes, but that is the interface for that operator.

-- 
Lawrence Crowl


Re: [cxx-conversion] Make double_int a class with methods and operators. (issue6443093)

2012-08-08 Thread Lawrence Crowl
On 8/7/12, Mike Stump  wrote:
> On Aug 7, 2012, at 11:42 AM, Lawrence Crowl wrote:
> > On 8/7/12, Mike Stump  wrote:
> > > On Aug 6, 2012, at 5:35 PM, Lawrence Crowl wrote:
> > > > Convert double_int from a struct with function into a class
> > > > with operators and methods.
> > >
> > > We have a wide_int class that replaces this class.  :-(
> >
> > Really?  Where?  I don't see neither definition nor use
> > under gcc.
>
> It is being developed; it is largely done, but is going through
> final reviews now.  When we finish with those review bits, we'll
> send it out.  It sounds like you'll beat us to the merge window,
> so, we'll cope with the fallout.

Can I have a pointer to the interface?

> > > It would have been better to just convert it.
> >
> > That depends on how easy it is to migrate.  In the process I
> > learned that double_int isn't actually a double int, but is a
> > precision-controlled int.
>
> Well, I'd call it a pair of HOST_WIDE_INTs...  I think calling
> it a precision-controlled int is a tad optimistic.  For example,
> if want to control it to be 2048 bits, you'd find that your hopes
> are misplaced.  :-)

Yes, well, it's clearly all relying on the host being bigger than
the target.

-- 
Lawrence Crowl


Re: [cxx-conversion] Make double_int a class with methods and operators. (issue6443093)

2012-08-08 Thread Gabriel Dos Reis
On Wed, Aug 8, 2012 at 3:29 AM, Miles Bader  wrote:
> Richard Guenther  writes:
> Constructors are allowed, but PODs are often passed more efficiently.
> That property seemed particularly important for double_int.

 Show us the difference in timing.  Show us the generated code.  I
 can't imagine that it could ever matter.
>>>
>>> I'm also curious about that statement...  PODs don't really seem to
>>> offer much advantage with modern compilers, except in a few very
>>> specific cases (of which this doesn't seem to be one), e.g. in unions.
>>
>> They make a difference for the by-value passing ABI.  double-ints can
>> be passed in two registers on most platforms.
>
> Sure, but that doesn't seem to depend on PODness -- non-PODs can be
> passed in two registers as well, AFAICS...
>
> E.g., in the following:
>
>typedef long valtype;
>
>struct X { valtype x, y; };
>struct Y { Y (valtype a, valtype b) : x (a), y (b) { } valtype x, y; };
>
>extern void fx (X x);
>void test_x () {X x = { 1, 2 }; fx (x); }
>
>extern void fy (Y y);
>void test_y () {Y y (1, 2); fy (y); }
>
> test_x and test_y use exactly the same calling sequence (and contain
> exactly the same assembly code)...  [on x86-64]

It is not PODness in the standard sense that matters.
It is podness from the ABI perspecitve: Y does not
have a user-defined copy-constructor nor a desctructor;
it is not a polymorphic type, so it is OK to pass in registers
just like X.

-- Gaby


Re: [cxx-conversion] Make double_int a class with methods and operators. (issue6443093)

2012-08-08 Thread Richard Guenther
On Wed, Aug 8, 2012 at 10:33 AM, Marc Glisse  wrote:
> On Wed, 8 Aug 2012, Richard Guenther wrote:
>
> +  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);
>
> [...]
>
 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.
>
>
> Sadly, that doesn't work with the current C++ rules (there is a proposal to
> change that for C++1y):
>
> void f(long);
> void f(unsigned long);
> void g(int x){f(x);}
>
> e.cc: In function ‘void g(int)’:
> e.cc:3:18: error: call of overloaded ‘f(int&)’ is ambiguous
> e.cc:3:18: note: candidates are:
> e.cc:1:6: note: void f(long int)
> e.cc:2:6: note: void f(long unsigned int)

Ick ... I forgot that integer promotions do not apply for int -> long.
 So even f(1) would
be ambiguous, right?  So I suppose we have to bite the bullet and add overloads
for all (unsigned) integer types from int to HOST_WIDE_INT (if HOST_WIDE_INT
is long long then we have to add overloads for int, unsigned int, long and
unsigned long).

Or use template magic to force promotion to HOST_WIDE_INT for integer
types smaller than HOST_WIDE_INT...

Richard.

> --
> Marc Glisse


Re: [cxx-conversion] Make double_int a class with methods and operators. (issue6443093)

2012-08-08 Thread Marc Glisse

On Wed, 8 Aug 2012, Richard Guenther wrote:


+  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);

[...]

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.


Sadly, that doesn't work with the current C++ rules (there is a proposal 
to change that for C++1y):


void f(long);
void f(unsigned long);
void g(int x){f(x);}

e.cc: In function ‘void g(int)’:
e.cc:3:18: error: call of overloaded ‘f(int&)’ is ambiguous
e.cc:3:18: note: candidates are:
e.cc:1:6: note: void f(long int)
e.cc:2:6: note: void f(long unsigned int)

--
Marc Glisse


Re: [cxx-conversion] Make double_int a class with methods and operators. (issue6443093)

2012-08-08 Thread Miles Bader
Richard Guenther  writes:
 Constructors are allowed, but PODs are often passed more efficiently.
 That property seemed particularly important for double_int.
>>>
>>> Show us the difference in timing.  Show us the generated code.  I
>>> can't imagine that it could ever matter.
>>
>> I'm also curious about that statement...  PODs don't really seem to
>> offer much advantage with modern compilers, except in a few very
>> specific cases (of which this doesn't seem to be one), e.g. in unions.
>
> They make a difference for the by-value passing ABI.  double-ints can
> be passed in two registers on most platforms.

Sure, but that doesn't seem to depend on PODness -- non-PODs can be
passed in two registers as well, AFAICS...

E.g., in the following:

   typedef long valtype;

   struct X { valtype x, y; };
   struct Y { Y (valtype a, valtype b) : x (a), y (b) { } valtype x, y; };

   extern void fx (X x);
   void test_x () {X x = { 1, 2 }; fx (x); }

   extern void fy (Y y);
   void test_y () {Y y (1, 2); fy (y); }

test_x and test_y use exactly the same calling sequence (and contain
exactly the same assembly code)...  [on x86-64]

Thanks,

-miles

-- 
80% of success is just showing up.  --Woody Allen


Re: [cxx-conversion] Make double_int a class with methods and operators. (issue6443093)

2012-08-08 Thread Richard Guenther
On Tue, Aug 7, 2012 at 8:38 PM, Lawrence Crowl  wrote:
> On 8/7/12, Richard Guenther  wrote:
>> On Tue, Aug 7, 2012 at 2:35 AM, Lawrence Crowl  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

Re: [cxx-conversion] Make double_int a class with methods and operators. (issue6443093)

2012-08-08 Thread Richard Guenther
On Wed, Aug 8, 2012 at 9:09 AM, Miles Bader  wrote:
> Mike Stump  writes:
>>> Constructors are allowed, but PODs are often passed more efficiently.
>>> That property seemed particularly important for double_int.
>>
>> Show us the difference in timing.  Show us the generated code.  I
>> can't imagine that it could ever matter.
>
> I'm also curious about that statement...  PODs don't really seem to
> offer much advantage with modern compilers, except in a few very
> specific cases (of which this doesn't seem to be one), e.g. in unions.

They make a difference for the by-value passing ABI.  double-ints can
be passed in two registers on most platforms.

Richard.

> -miles
>
> --
> values of β will give rise to dom!


Re: [cxx-conversion] Make double_int a class with methods and operators. (issue6443093)

2012-08-08 Thread Miles Bader
Mike Stump  writes:
>> Constructors are allowed, but PODs are often passed more efficiently.
>> That property seemed particularly important for double_int.
>
> Show us the difference in timing.  Show us the generated code.  I
> can't imagine that it could ever matter.

I'm also curious about that statement...  PODs don't really seem to
offer much advantage with modern compilers, except in a few very
specific cases (of which this doesn't seem to be one), e.g. in unions.

-miles

-- 
values of β will give rise to dom!


Re: [cxx-conversion] Make double_int a class with methods and operators. (issue6443093)

2012-08-07 Thread Richard Henderson
On 08/06/2012 05:35 PM, Lawrence Crowl wrote:
> +inline double_int &
> +double_int::operator ++ ()
> +{
> +  *this + double_int_one;
> +  return *this;
> +}
> +
> +inline double_int &
> +double_int::operator -- ()
> +{
> +  *this - double_int_one;
> +  return *this;
> +}

Surely unused results there?


r~


Re: [cxx-conversion] Make double_int a class with methods and operators. (issue6443093)

2012-08-07 Thread Mike Stump
On Aug 7, 2012, at 11:42 AM, Lawrence Crowl wrote:
> On 8/7/12, Mike Stump  wrote:
>> On Aug 6, 2012, at 5:35 PM, Lawrence Crowl wrote:
>>> Convert double_int from a struct with function into a class with
>>> operators and methods.
>> 
>> We have a wide_int class that replaces this class.  :-(
> 
> Really?  Where?  I don't see neither definition nor use under gcc.

It is being developed; it is largely done, but is going through final reviews 
now.  When we finish with those review bits, we'll send it out.  It sounds like 
you'll beat us to the merge window, so, we'll cope with the fallout.

>> It would have been better to just convert it.
> 
> That depends on how easy it is to migrate.  In the process I
> learned that double_int isn't actually a double int, but is a
> precision-controlled int.

Well, I'd call it a pair of HOST_WIDE_INTs...  I think calling it a 
precision-controlled int is a tad optimistic.  For example, if want to control 
it to be 2048 bits, you'd find that your hopes are misplaced.  :-)


Re: [cxx-conversion] Make double_int a class with methods and operators. (issue6443093)

2012-08-07 Thread Mike Stump
On Aug 7, 2012, at 11:38 AM, Lawrence Crowl wrote:
> 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.

If there is an agreed upon and expected semantic, having them are useful.  In 
the wide-int world, which replaces double_int, I think there is an agreeable 
semantic and I think it is useful, so, I think we should plan on having them, 
though, I'd be fine with punting their implementation until such time as 
someone needs it.  If no one every needs the routine, I don't see the harm in 
not implementing it.

>> 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.

Show us the difference in timing.  Show us the generated code.  I can't imagine 
that it could ever matter.  I think a pretty abi is worth the cost.


Re: [cxx-conversion] Make double_int a class with methods and operators. (issue6443093)

2012-08-07 Thread Lawrence Crowl
On 8/7/12, Mike Stump  wrote:
> On Aug 6, 2012, at 5:35 PM, Lawrence Crowl wrote:
>> Convert double_int from a struct with function into a class with
>> operators and methods.
>
> We have a wide_int class that replaces this class.  :-(

Really?  Where?  I don't see neither definition nor use under gcc.

> It would have been better to just convert it.

That depends on how easy it is to migrate.  In the process I
learned that double_int isn't actually a double int, but is a
precision-controlled int.

-- 
Lawrence Crowl


Re: [cxx-conversion] Make double_int a class with methods and operators. (issue6443093)

2012-08-07 Thread Lawrence Crowl
On 8/7/12, Richard Guenther  wrote:
> On Tue, Aug 7, 2012 at 2:35 AM, Lawrence Crowl  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.

> >   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.

> > -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.

> 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.

-- 
Lawrence Crowl


Re: [cxx-conversion] Make double_int a class with methods and operators. (issue6443093)

2012-08-07 Thread Diego Novillo

On 12-08-07 13:22 , Mike Stump wrote:

On Aug 6, 2012, at 5:35 PM, Lawrence Crowl wrote:

Convert double_int from a struct with function into a class with
operators and methods.


We have a wide_int class that replaces this class.  :-(  It would
have been better to just convert it.  Do you guys have a timeframe
for the cxx-conversion landing?


Soon (for "soon" measured in days).  I will try to send the merge 
patches this week.



Diego.


Re: [cxx-conversion] Make double_int a class with methods and operators. (issue6443093)

2012-08-07 Thread Mike Stump
On Aug 6, 2012, at 5:35 PM, Lawrence Crowl wrote:
> Convert double_int from a struct with function into a class with
> operators and methods.

We have a wide_int class that replaces this class.  :-(  It would have been 
better to just convert it.  Do you guys have a timeframe for the cxx-conversion 
landing?


Re: [cxx-conversion] Make double_int a class with methods and operators. (issue6443093)

2012-08-07 Thread Richard Guenther
On Tue, Aug 7, 2012 at 2:35 AM, Lawrence Crowl  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.

>   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.

> -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 ...

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.  So as an intermediate
I'd like you _not_ to introduce the make () overloads.

Btw, if HOST_WIDE_INT == int the above won't even compile.

Richard.

> +  /* No copy assignment operator or destructor to keep the type a POD.  */
> +
> +  /* There are some special value-creation static member functions.  */
> +
> +  static double_int mask (unsigned prec);
> +  static double_int max_value (unsigned int prec, bool uns);
> +  static double_int min_value (unsigned int prec, bool uns);
> +
> +  /* The following functions are mutating operations.  */
> +
> +  double_int &operator ++(); // prefix
> +  double_int &operator --(); // prefix
> +  double_int &operator *= (double_int);
> +  double_int &operator += (double_int);
> +  double_int &operator -= (double_int);
> +
> +  /* The following functions are non-mutating operations.  */
> +
> +  /* Conversion functions.  */
> +
> +  HOST_WIDE_INT to_signed () const;
> +  unsigned HOST_WIDE_INT to_unsigned () const;
> +
> +  /* Conversion query functions.  */
> +
> +  bool fits_unsigned() const;
> +  bool fits_signed() const;
> +  bool fits (bool uns) const;
> +
> +  /* Attribute query functions.  */
> +
> +  int trailing_zeros () const;
> +  int popcount () const;
> +
> +  /* Arithmetic query operations.  */
> +
> +  bool multiple_of (double_int, bool, double_int *) const;
> +
> +  /* Arithmetic operation functions.  */
> +
> +  double_int set_bit (unsigned) const;
> +  double_int mul_with_sign (double_int, bool, int *) const;
> +
> +  double_int operator * (double_int b) const;
> +  double_int operator + (double_int b) const;
> +  double_int operator - (double_int b) const;
> +  double_int operator - () const;
> +  double_int operator ~ () const;
> +  double_int operator & (double_int b) const;
> +  double_int operator | (double_int b) const;
> +  d