cfgexpand.c patch for [was new port: msp430-elf]

> > Note that I had to make a few changes (fixes?)  in the MI portions of
> > gcc to avoid problems I encountered, I don't know if these changes are
> > "correct" or if there are better ways to avoid those cases.  Those
> 
> In any case, they should best be posted in separate messages, each one 
> with its own rationale.

Here's the first of those...  The patch assumes that, "by definition",
a partial int mode has fewer bits than an int mode of the same size,
and thus truncation should be used to go from the int mode to the
partial int mode.

On Sat, May 11, 2013 at 1:41 AM, DJ Delorie <dj@redhat.com> wrote:
>
>> > Note that I had to make a few changes (fixes?)  in the MI portions of
>> > gcc to avoid problems I encountered, I don't know if these changes are
>> > "correct" or if there are better ways to avoid those cases.  Those
>>
>> In any case, they should best be posted in separate messages, each one
>> with its own rationale.
>
> Here's the first of those...  The patch assumes that, "by definition",
> a partial int mode has fewer bits than an int mode of the same size,
> and thus truncation should be used to go from the int mode to the
> partial int mode.

Can you add that (partial int modes have fewer bits than int modes) as
verification
to genmodes.c:make_partial_integer_mode?

> Index: gcc/cfgexpand.c
> ===================================================================
> --- gcc/cfgexpand.c     (revision 198591)
> +++ gcc/cfgexpand.c     (working copy)
> @@ -3090,13 +3090,17 @@ expand_debug_expr (tree exp)
>          size_t, we need to check for mis-matched modes and correct
>          the addend.  */
>        if (op0 && op1
>           && GET_MODE (op0) != VOIDmode && GET_MODE (op1) != VOIDmode
>           && GET_MODE (op0) != GET_MODE (op1))
>         {
> -         if (GET_MODE_BITSIZE (GET_MODE (op0)) < GET_MODE_BITSIZE (GET_MODE (op1)))

I wonder if this should not use GET_MODE_PRECISION - after all it is
the precision that determines whether we have to extend / truncate?  Or
is precision a so much unused term on RTL that this would cause problems?
Thus,

> +         if (GET_MODE_BITSIZE (GET_MODE (op0)) < GET_MODE_BITSIZE (GET_MODE (op1))

             if (GET_MODE_PRECISION (GET_MODE (op0)) <
GET_MODE_PRECISION (GET_MODE (op1)))
               op1 = simplify_gen_unary (TRUNCATE, GET_MODE (op0),
op1, GET_MODE (op1));

?

Richard.

> +             /* Don't try to sign-extend SImode to PSImode, for example.  */
> +             || (GET_MODE_BITSIZE (GET_MODE (op0)) == GET_MODE_BITSIZE (GET_MODE (op1))
> +                 && GET_MODE_CLASS (GET_MODE (op0)) == MODE_PARTIAL_INT
> +                 && GET_MODE_CLASS (GET_MODE (op1)) == MODE_INT))
>             op1 = simplify_gen_unary (TRUNCATE, GET_MODE (op0), op1,
>                                       GET_MODE (op1));
>           else
>             /* We always sign-extend, regardless of the signedness of
>                the operand, because the operand is always unsigned
>                here even if the original C expression is signed.  */

> Can you add that (partial int modes have fewer bits than int modes)
> as verification to genmodes.c:make_partial_integer_mode?

I could, but it would be a no-op for PARTIAL_INT_MODE()

> I wonder if this should not use GET_MODE_PRECISION - after all it is
> the precision that determines whether we have to extend / truncate?
> Or is precision a so much unused term on RTL that this would cause
> problems?

The problem is, the precision of PSImode *is* the same as SImode,
if you just use PARTIAL_INT_MODE() in *-modes.def

On Mon, May 13, 2013 at 7:23 PM, DJ Delorie <dj@redhat.com> wrote:
>
>> Can you add that (partial int modes have fewer bits than int modes)
>> as verification to genmodes.c:make_partial_integer_mode?
>
> I could, but it would be a no-op for PARTIAL_INT_MODE()
>
>> I wonder if this should not use GET_MODE_PRECISION - after all it is
>> the precision that determines whether we have to extend / truncate?
>> Or is precision a so much unused term on RTL that this would cause
>> problems?
>
> The problem is, the precision of PSImode *is* the same as SImode,
> if you just use PARTIAL_INT_MODE() in *-modes.def

How can you then ever "truncate" from SImode to PSImode?  That is,
currently we have

          if (GET_MODE_BITSIZE (GET_MODE (op0)) < GET_MODE_BITSIZE
(GET_MODE (op1)))
            op1 = simplify_gen_unary (TRUNCATE, GET_MODE (op0), op1,
                                      GET_MODE (op1));
          else
            /* We always sign-extend, regardless of the signedness of
               the operand, because the operand is always unsigned
               here even if the original C expression is signed.  */
            op1 = simplify_gen_unary (SIGN_EXTEND, GET_MODE (op0), op1,
                                      GET_MODE (op1));

but the case of same precision/bitsize is not handled here.  So, shouldn't it
be then

           if (GET_MODE_BITSIZE (GET_MODE (op0)) == GET_MODE_BITSIZE
(GET_MODE (op1)))
             op1 = convert_move (GET_MODE (op0), op1);
           else if (....

?

That said, special casing PARTIAL_INT_MODEs anywhere looks bogus to me.
Which might mean that PARTIAL_INT_MODEs are bogus... what are they
useful for if they do not even have a precision and share the same bitsize
as their base mode?  Do they even have a "precision"?

> grep PARTIAL_INT_MODE config/*/*.def
config/bfin/bfin-modes.def:PARTIAL_INT_MODE (DI);
config/m32c/m32c-modes.def:PARTIAL_INT_MODE (SI);
config/rs6000/rs6000-modes.def:PARTIAL_INT_MODE (TI);
config/sh/sh-modes.def:PARTIAL_INT_MODE (SI);
config/sh/sh-modes.def:PARTIAL_INT_MODE (DI);

so it shouldn't be difficult to fix that precision thing by adjusting genmodes.c
and the few occurences above?  Make it

config/bfin/bfin-modes.def:PARTIAL_INT_MODE (DI, 40);

btw, what's the relation to fractional int modes?

Richard.

> How can you then ever "truncate" from SImode to PSImode?

If you use PARTIAL_INT_MODE(), you get a PSImode that has a "default"
bitsize (i.e. the value stored in the data structure) that's the same
as SImode, that is, 32.  There is no way to specify the usable
bitsize, so it's "undefined/unspecified" but, IMHO, assumed to be less
than the bit size of the parent mode.

One can assert that a PSImode will never have *more* precision than
SImode, though, so sign extending SImode to PSImode is never the right
thing to do.

> btw, what's the relation to fractional int modes?

I think fractional_int_mode allows you to specify the precision and
byte size, but in my experience, I've had trouble using it, especially
if you specify a non-power-of-two byte size.

On Tue, May 14, 2013 at 6:03 PM, DJ Delorie <dj@redhat.com> wrote:
>
>> How can you then ever "truncate" from SImode to PSImode?
>
> If you use PARTIAL_INT_MODE(), you get a PSImode that has a "default"
> bitsize (i.e. the value stored in the data structure) that's the same
> as SImode, that is, 32.  There is no way to specify the usable
> bitsize, so it's "undefined/unspecified" but, IMHO, assumed to be less
> than the bit size of the parent mode.

My question is, if you end up with

  (truncate:PSI (reg:SI 27))

and then constant propagate 0x7fffffff to reg:SI 27, what does simplify-rtx.c
do here?  Truncate to _what_ precision exactly?

Recent introduction of PTImode to rs6000 makes me think that the
PARTIAL_INT_MODE()s are a hack to simply get another name for
TImode (in this case).

Thus to the middle-end it shouldn't be

  (truncate:PSI (reg:SI 27))

but

  (set (reg:PSI 28 (reg:SI 27)))

or maybe

  (subreg:PSI (reg:SI 27))

?

> One can assert that a PSImode will never have *more* precision than
> SImode, though, so sign extending SImode to PSImode is never the right
> thing to do.
>
>> btw, what's the relation to fractional int modes?
>
> I think fractional_int_mode allows you to specify the precision and
> byte size, but in my experience, I've had trouble using it, especially
> if you specify a non-power-of-two byte size.

Interestingly we have exactly that for AVR:

config/avr/avr-modes.def:FRACTIONAL_INT_MODE (PSI, 24, 3);

and they call it PSImode ;)

So ... time to remove PARTIAL_INT_MODE ()s?  Btw, I wonder why

INT_MODE (PSI, 4)

would not work equally well as

PARTIAL_INT_MODE (SI)

?

Richard.

On May 15, 2013, at 1:27 AM, Richard Biener <richard.guenther@gmail.com> wrote:
> My question is, if you end up with
> 
>  (truncate:PSI (reg:SI 27))
> 
> and then constant propagate 0x7fffffff to reg:SI 27, what does simplify-rtx.c
> do here?  Truncate to _what_ precision exactly?

In my world, I change PSI to be P28SI, and then answer is that there are 28 bits.  All ports that create partial int modes full well know the exact precision, and that can be added.  There is a ton of support already in the compiler for this, and the last little bit in the mode def language and to strap it in is light weight and obvious.

> Recent introduction of PTImode to rs6000 makes me think that the
> PARTIAL_INT_MODE()s are a hack to simply get another name for
> TImode (in this case).
> 
> Thus to the middle-end it shouldn't be
> 
>  (truncate:PSI (reg:SI 27))
> 
> but
> 
>  (set (reg:PSI 28 (reg:SI 27)))
> 
> or maybe
> 
>  (subreg:PSI (reg:SI 27))

I see all forms as valid, but not the same.

(set (reg:P28SI (reg:SI 29))
       (truncate:P28SI (reg:SI 27)))

is natural and reasonable, which combines two of the forms above.  Using truncate is fine.

> config/avr/avr-modes.def:FRACTIONAL_INT_MODE (PSI, 24, 3);

I never got any joy from FRACTIONAL_INT_MODE.

> So ... time to remove PARTIAL_INT_MODE ()s?  Btw, I wonder why

No.

On Wed, May 15, 2013 at 7:24 PM, Mike Stump <mrs@mrs.kithrup.com> wrote:
> On May 15, 2013, at 1:27 AM, Richard Biener <richard.guenther@gmail.com> wrote:
>> My question is, if you end up with
>>
>>  (truncate:PSI (reg:SI 27))
>>
>> and then constant propagate 0x7fffffff to reg:SI 27, what does simplify-rtx.c
>> do here?  Truncate to _what_ precision exactly?
>
> In my world, I change PSI to be P28SI, and then answer is that there are 28 bits.  All ports that create partial int modes full well know the exact precision, and that can be added.  There is a ton of support already in the compiler for this, and the last little bit in the mode def language and to strap it in is light weight and obvious.

Indeed.  What's the blocker to convert the existing 5 cases of PARTIAL_INT_MODE
use to specify a precision?

Richard.

>> Recent introduction of PTImode to rs6000 makes me think that the
>> PARTIAL_INT_MODE()s are a hack to simply get another name for
>> TImode (in this case).
>>
>> Thus to the middle-end it shouldn't be
>>
>>  (truncate:PSI (reg:SI 27))
>>
>> but
>>
>>  (set (reg:PSI 28 (reg:SI 27)))
>>
>> or maybe
>>
>>  (subreg:PSI (reg:SI 27))
>
> I see all forms as valid, but not the same.
>
> (set (reg:P28SI (reg:SI 29))
>        (truncate:P28SI (reg:SI 27)))
>
> is natural and reasonable, which combines two of the forms above.  Using truncate is fine.
>
>> config/avr/avr-modes.def:FRACTIONAL_INT_MODE (PSI, 24, 3);
>
> I never got any joy from FRACTIONAL_INT_MODE.
>
>> So ... time to remove PARTIAL_INT_MODE ()s?  Btw, I wonder why
>
> No.

> What's the blocker to convert the existing 5 cases of
> PARTIAL_INT_MODE use to specify a precision?

In general?  For me, PARTIAL_INT_MODE() works, FRACTIONAL_INT_MODE()
didn't.

> Interestingly we have exactly that for AVR:
> 
> config/avr/avr-modes.def:FRACTIONAL_INT_MODE (PSI, 24, 3);

I know.  I tried copying them, it didn't work for me.

On Thu, 16 May 2013, DJ Delorie wrote:

> > What's the blocker to convert the existing 5 cases of
> > PARTIAL_INT_MODE use to specify a precision?
> 
> In general?  For me, PARTIAL_INT_MODE() works, FRACTIONAL_INT_MODE()
> didn't.

I thought it was the other way round - that after Bernd's fixes (July 
2011) towards support for 40-bit integers, FRACTIONAL_INT_MODE worked 
better than PARTIAL_INT_MODE.

> I thought it was the other way round - that after Bernd's fixes (July 
> 2011) towards support for 40-bit integers, FRACTIONAL_INT_MODE worked 
> better than PARTIAL_INT_MODE.

Probably most accurate to say that both ways are not well supported.
In general, I always have a hard time with anything that isn't a
power-of-two size in gcc, because most maintainers have no reason to
even consider such cases.  There are even some fundamantal problems
I've run into, for example...

SIZE_TYPE is compared - with individual strcmp's! - against a fixed
set of C types, instead of using some sort of table lookup to support
non-power-of-two types.

P*type modes are not stored in the modes table with *type modes, but
the functions to get the "next bigger mode" blindly assumes that
mode[N+1] is the next mode, which ignores partial int modes
completely.

Operations with 3-byte PSImode end up using BLKmode sometimes.

On Thu, 16 May 2013, DJ Delorie wrote:

> SIZE_TYPE is compared - with individual strcmp's! - against a fixed
> set of C types, instead of using some sort of table lookup to support
> non-power-of-two types.

Joern had a patch (as of December 2010, possibly updated since then) to 
convert at least some such macros to use enum values (stdint.h ones, 
anyway) to use enum values rather than strings as a cleaner API.

Though that wouldn't be sufficient for what you want.  The front-end type 
pieces of Bernd's changes didn't go in, since they embedded things 
specific to 40-bit types all over the place rather than providing a more 
general infrastructure for types of architecture-specific precision.  
Logically I'd like a way for an architecture to define a set of precisions 
for which there are __intN keywords and associated __intN, unsigned __intN 
types (plus the _Complex versions of those).  Then maybe the values given 
for types such as SIZE_TYPE would be either an enum value for a standard 
type, or e.g. ITK_INTN (40) or ITK_UNSIGNED_INTN (40), where ITK_INTN 
expands to some expression giving a value of the enum type not 
corresponding to any of the standard named types.

Likewise for floating-point types.  DTS 18661-3 (the third part of the 
draft ISO C bindings to IEEE 754-2008) defines types

_FloatN, where N is 16, 32, 64, or >= 128 and a multiple of 32
_DecimalN, where N >= 32 and a multiple of 32

and _Complex variants of the _FloatN types, where the particular set 
supported depends on the implementation (and so for GCC would depend on 
the architecture).  Again, these should be keywords.

(I don't think any of the above should be particularly hard to implement.)