Re: [wide-int] Make trees more like rtxes
On Wed, 23 Oct 2013, Richard Sandiford wrote: > Mike Stump writes: > > On Oct 23, 2013, at 5:00 AM, Richard Sandiford > > wrote: > >> offset_int, max_int, wi::to_offset and wi::to_max sound OK to me. > >> Kenny? Mike? > > > > Those two names seem reasonable. to_offset should document that these > > are for address offsets (and address constants) exclusively. > > Reading this back, I realise "max_int" might sound too similar to INT_MAX. > Maybe we could follow the current HOST_* stuff and use: offset_int, > widest_int, > wi::to_offset and wi::to_widest. > > Bah. I'm no good at naming stuff... Nobody is ... but yes, offset_int and widest_int and wi::to_offset and wi::to_widest sounds good to me - both short and descriptive. Richard.
Re: [wide-int] Make trees more like rtxes
Mike Stump writes: > On Oct 23, 2013, at 5:00 AM, Richard Sandiford > wrote: >> offset_int, max_int, wi::to_offset and wi::to_max sound OK to me. >> Kenny? Mike? > > Those two names seem reasonable. to_offset should document that these > are for address offsets (and address constants) exclusively. Reading this back, I realise "max_int" might sound too similar to INT_MAX. Maybe we could follow the current HOST_* stuff and use: offset_int, widest_int, wi::to_offset and wi::to_widest. Bah. I'm no good at naming stuff... Richard
Re: [wide-int] Make trees more like rtxes
On Oct 23, 2013, at 5:00 AM, Richard Sandiford wrote: > offset_int, max_int, wi::to_offset and wi::to_max sound OK to me. > Kenny? Mike? Those two names seem reasonable. to_offset should document that these are for address offsets (and address constants) exclusively.
Re: [wide-int] Make trees more like rtxes
On Oct 23, 2013, at 2:09 AM, Richard Biener wrote: > Good. Better names - ah well, wi::to_max_wide_int (t) and > wi::to_addr_wide_int (t)? Btw, "addr_wide_int" is an odd name The idea was to have one type to rule them all… everything address related... bit offsets, byte offsets… Bit offset is the wrong name, as that would be a wrong type to use for a byte offset. Someone that has a bit offset type, would naturally want to /8 to get a byte offset. If one wanted a bit offset, then it should be split into two, bit offset and byte offset.
Re: [wide-int] Make trees more like rtxes
On 10/23/2013 08:13 AM, Richard Biener wrote:
On Wed, 23 Oct 2013, Richard Sandiford wrote:
Richard Biener writes:
The patch does that by adding:
wi::address (t)
for when we want to extend tree t to addr_wide_int precision and:
wi::extend (t)
for when we want to extend it to max_wide_int precision. (Better names
welcome.) These act just like addr_wide_int (t) and max_wide_int (t)
would on current sources, except that they use the tree representation
directly, so there's no copying.
Good. Better names - ah well, wi::to_max_wide_int (t) and
wi::to_addr_wide_int (t)? Btw, "addr_wide_int" is an odd name as it
has at least the precision of the maximum _bit_ offset possible, right?
So more like [bit_]offset_wide_int? Or just [bit_]offset_int?
And then wi::to_offset (t) and wi::to_max (t)?
offset_int, max_int, wi::to_offset and wi::to_max sound OK to me.
Kenny? Mike?
Most of the patch is mechanical and many of the "wi::address (...)"s
and "wi::extend (...)"s reinstate "addr_wide_int (...)"s and
"max_wide_int (...)"s from the initial implementation. Sorry for the
run-around on this.
One change I'd like to point out though is:
@@ -7287,7 +7287,9 @@ native_encode_int (const_tree expr, unsi
for (byte = 0; byte < total_bytes; byte++)
{
int bitpos = byte * BITS_PER_UNIT;
- value = wi::extract_uhwi (expr, bitpos, BITS_PER_UNIT);
+ /* Extend EXPR according to TYPE_SIGN if the precision isn't a whole
+number of bytes. */
+ value = wi::extract_uhwi (wi::extend (expr), bitpos, BITS_PER_UNIT);
if (total_bytes > UNITS_PER_WORD)
{
I think this preserves the existing trunk behaviour but I wasn't sure
whether it was supposed to work like that or whether upper bits should
be zero.
I think the upper bits are undefined, the trunk native_interpret_int
does
result = double_int::from_buffer (ptr, total_bytes);
return double_int_to_tree (type, result);
where the call to double_int_to_tree re-extends according to the types
precision and sign. wide_int_to_tree doesn't though?
This is native_encode_int rather than native_interpret_int though.
Yes, I was looking at the matched interpret variant though to see
what we do.
the wide_int_to_tree really needs to canonicalize the value before
making it into a tree.
the calls to tree_fits*_p (the successor to host_integer_p) depend on
this being clean.
Otherwise, these functions will have to clean the short integers and
they get called all over the place.
AIUI it's used for VIEW_CONVERT_EXPRs, so I thought the upper bits
might get used.
Yeah, that might happen, but still relying on the upper bits in any
way would be brittle here.
Richard.
Re: [wide-int] Make trees more like rtxes
On Wed, 23 Oct 2013, Richard Sandiford wrote:
> Richard Biener writes:
> >> The patch does that by adding:
> >>
> >> wi::address (t)
> >>
> >> for when we want to extend tree t to addr_wide_int precision and:
> >>
> >> wi::extend (t)
> >>
> >> for when we want to extend it to max_wide_int precision. (Better names
> >> welcome.) These act just like addr_wide_int (t) and max_wide_int (t)
> >> would on current sources, except that they use the tree representation
> >> directly, so there's no copying.
> >
> > Good. Better names - ah well, wi::to_max_wide_int (t) and
> > wi::to_addr_wide_int (t)? Btw, "addr_wide_int" is an odd name as it
> > has at least the precision of the maximum _bit_ offset possible, right?
> > So more like [bit_]offset_wide_int? Or just [bit_]offset_int?
> > And then wi::to_offset (t) and wi::to_max (t)?
>
> offset_int, max_int, wi::to_offset and wi::to_max sound OK to me.
> Kenny? Mike?
>
> >> Most of the patch is mechanical and many of the "wi::address (...)"s
> >> and "wi::extend (...)"s reinstate "addr_wide_int (...)"s and
> >> "max_wide_int (...)"s from the initial implementation. Sorry for the
> >> run-around on this.
> >>
> >> One change I'd like to point out though is:
> >>
> >> @@ -7287,7 +7287,9 @@ native_encode_int (const_tree expr, unsi
> >>for (byte = 0; byte < total_bytes; byte++)
> >> {
> >>int bitpos = byte * BITS_PER_UNIT;
> >> - value = wi::extract_uhwi (expr, bitpos, BITS_PER_UNIT);
> >> + /* Extend EXPR according to TYPE_SIGN if the precision isn't a whole
> >> + number of bytes. */
> >> + value = wi::extract_uhwi (wi::extend (expr), bitpos, BITS_PER_UNIT);
> >>
> >>if (total_bytes > UNITS_PER_WORD)
> >>{
> >>
> >> I think this preserves the existing trunk behaviour but I wasn't sure
> >> whether it was supposed to work like that or whether upper bits should
> >> be zero.
> >
> > I think the upper bits are undefined, the trunk native_interpret_int
> > does
> >
> > result = double_int::from_buffer (ptr, total_bytes);
> >
> > return double_int_to_tree (type, result);
> >
> > where the call to double_int_to_tree re-extends according to the types
> > precision and sign. wide_int_to_tree doesn't though?
>
> This is native_encode_int rather than native_interpret_int though.
Yes, I was looking at the matched interpret variant though to see
what we do.
> AIUI it's used for VIEW_CONVERT_EXPRs, so I thought the upper bits
> might get used.
Yeah, that might happen, but still relying on the upper bits in any
way would be brittle here.
Richard.
Re: [wide-int] Make trees more like rtxes
Richard Biener writes:
>> The patch does that by adding:
>>
>> wi::address (t)
>>
>> for when we want to extend tree t to addr_wide_int precision and:
>>
>> wi::extend (t)
>>
>> for when we want to extend it to max_wide_int precision. (Better names
>> welcome.) These act just like addr_wide_int (t) and max_wide_int (t)
>> would on current sources, except that they use the tree representation
>> directly, so there's no copying.
>
> Good. Better names - ah well, wi::to_max_wide_int (t) and
> wi::to_addr_wide_int (t)? Btw, "addr_wide_int" is an odd name as it
> has at least the precision of the maximum _bit_ offset possible, right?
> So more like [bit_]offset_wide_int? Or just [bit_]offset_int?
> And then wi::to_offset (t) and wi::to_max (t)?
offset_int, max_int, wi::to_offset and wi::to_max sound OK to me.
Kenny? Mike?
>> Most of the patch is mechanical and many of the "wi::address (...)"s
>> and "wi::extend (...)"s reinstate "addr_wide_int (...)"s and
>> "max_wide_int (...)"s from the initial implementation. Sorry for the
>> run-around on this.
>>
>> One change I'd like to point out though is:
>>
>> @@ -7287,7 +7287,9 @@ native_encode_int (const_tree expr, unsi
>>for (byte = 0; byte < total_bytes; byte++)
>> {
>>int bitpos = byte * BITS_PER_UNIT;
>> - value = wi::extract_uhwi (expr, bitpos, BITS_PER_UNIT);
>> + /* Extend EXPR according to TYPE_SIGN if the precision isn't a whole
>> + number of bytes. */
>> + value = wi::extract_uhwi (wi::extend (expr), bitpos, BITS_PER_UNIT);
>>
>>if (total_bytes > UNITS_PER_WORD)
>> {
>>
>> I think this preserves the existing trunk behaviour but I wasn't sure
>> whether it was supposed to work like that or whether upper bits should
>> be zero.
>
> I think the upper bits are undefined, the trunk native_interpret_int
> does
>
> result = double_int::from_buffer (ptr, total_bytes);
>
> return double_int_to_tree (type, result);
>
> where the call to double_int_to_tree re-extends according to the types
> precision and sign. wide_int_to_tree doesn't though?
This is native_encode_int rather than native_interpret_int though.
AIUI it's used for VIEW_CONVERT_EXPRs, so I thought the upper bits
might get used.
Thanks,
Richard
