pre_inc/pre_dec/PUSH_ROUNDING inconsistency

pre_inc/pre_dec/PUSH_ROUNDING inconsistency

[Jan Hubicka]

Hi,
It seems to me, that there is imporntant inconcistency between pre_inc/pre_dec
operators and the PUSH_ROUNDING macro. The documentation says:
`(pre_dec:M X)'
     Represents the side effect of decrementing X by a standard amount
     and represents also the value that X has after being decremented.
     X must be a `reg' or `mem', but most machines allow only a `reg'.
     M must be the machine mode for pointers on the machine in use.
     The amount X is decremented by is the length in bytes of the
     machine mode of the containing memory reference of which this
     expression serves as the address.  Here is an example of its use:

          (mem:DF (pre_dec:SI (reg:SI 39)))

     This says to decrement pseudo register 39 by the length of a
     `DFmode' value and use the result to address a `DFmode' value.

so it seems to suggest, that pre_dec always decrements by the size of mode. On
the other hand, when I define PUSH_ROUDNING to round up everything to 8 (as I
do in md file I am working on), since I want all arguments to be aligned by
that amount and I don't want calls.c to emit stack pointer arithmetics for
pading of 32bit 'int' arguments, I get following instructions to push
32bit quantities (these are constructed mostly by emit_push_insn and are
out of scope of machine description, that gets just those operands to move
pattern):

(set (mem/f:SI (pre_dec:DI (reg:DI 7 sp)) (const_int 1)))

and code handling function calls expect the to be "push" operation and thus
adjust stack by PUSH_ROUNDING (4) size (that is 8),
but since mem mode is SImode, other parts of compiler (such as
combine_stack_adjustements) expect the sp value to be decremended by
size of SImode, thus 4 causing many missoptimizations.

What do you think is the correct way off this situation? Modify the
emit_push_insn in explow.c to change push insn representation for example
to something like:
(set (mem/f:DI (pre_dec:DI (reg:DI sp)) (subreg:DI val)))
since thats what the instruction does?
(can I easilly convert number of bytes returned by push_rounding to mode?)
or some other solution? (I guess it is not good idea to convert rest
of compiler to use PUSH_ROUDNING macro for autoinc/dec arithmetics).

Quick solution would be probably to promote all operands to DImodes, but
I would like to avoid zero/sign extending on caller side and I would run
into problems with XFmodes anyway - I see that the proposed push instruction
format will not work eighter - I can't subreg the XFmode to TFmode.
Implementing pushes in a way as pops are handled in i386.md (i.e as parallels
w/o the pre_dec operands) can be way, but it is quite nasty too, sicne
we will have to teach generic code, such as combine_stack_adjustements
to discover these as push insns.


Honza

Re: pre_inc/pre_dec/PUSH_ROUNDING inconsistency

[Richard Henderson]

On Sun, Jul 16, 2000 at 04:04:20PM +0200, Jan Hubicka wrote:
> What do you think is the correct way off this situation? Modify the
> emit_push_insn in explow.c to change push insn representation for example
> to something like:
> (set (mem/f:DI (pre_dec:DI (reg:DI sp)) (subreg:DI val)))
> since thats what the instruction does?

I would think that widening the store like this to reflect the
actual instruction is the only correct solution.

> (can I easilly convert number of bytes returned by push_rounding to mode?)

See smallest_mode_for_size.


r~

Re: pre_inc/pre_dec/PUSH_ROUNDING inconsistency

[Jan Hubicka]

> On Sun, Jul 16, 2000 at 04:04:20PM +0200, Jan Hubicka wrote:
> > What do you think is the correct way off this situation? Modify the
> > emit_push_insn in explow.c to change push insn representation for example
> > to something like:
> > (set (mem/f:DI (pre_dec:DI (reg:DI sp)) (subreg:DI val)))
> > since thats what the instruction does?
> 
> I would think that widening the store like this to reflect the
> actual instruction is the only correct solution.
Yes, but whats about the floating point stores. I remember Jeff claiming
that (subreg:TF (reg:XF)) is incorrect...
is (subreg:TI (reg:XF)) better?
> 
> > (can I easilly convert number of bytes returned by push_rounding to mode?)
> 
> See smallest_mode_for_size.
Duh :)

Honza
> 
> 
> r~

Re: pre_inc/pre_dec/PUSH_ROUNDING inconsistency

[Richard Henderson]

On Mon, Jul 17, 2000 at 11:10:40PM +0200, Jan Hubicka wrote:
> Yes, but whats about the floating point stores. I remember Jeff claiming
> that (subreg:TF (reg:XF)) is incorrect...
> is (subreg:TI (reg:XF)) better?

Yes, the TFmode subreg is incorrect.  As for the other... well,
I guess it's not too bad.  I'm not sure what to put in its place.



r~

Re: pre_inc/pre_dec/PUSH_ROUNDING inconsistency

[Jan Hubicka]

> On Mon, Jul 17, 2000 at 11:10:40PM +0200, Jan Hubicka wrote:
> > Yes, but whats about the floating point stores. I remember Jeff claiming
> > that (subreg:TF (reg:XF)) is incorrect...
> > is (subreg:TI (reg:XF)) better?
> 
> Yes, the TFmode subreg is incorrect.  As for the other... well,
> I guess it's not too bad.  I'm not sure what to put in its place.
OK. I will go with the integer subregs (it will require some care
in machine description, since integer pushdi instruction will be
receiving floats too... looks quite ugly.
It can also force reload to move value from FP reigsters, not being able
to hold TImode to integer before actually pushing. :(
Also it will probably inhibit the memory operand for such push instructions.
While most architectures are probably not crazy enought to define
"zero extend and push from memory" instruction, one I am working at
can handle such stuff by doing actually two instruction, one for data
and one for padding, saving an register.

Honza
> 
> 
> 
> r~

Re: pre_inc/pre_dec/PUSH_ROUNDING inconsistency

[Richard Henderson]

On Mon, Jul 17, 2000 at 02:16:04PM -0700, Richard Henderson wrote:
> Yes, the TFmode subreg is incorrect.  As for the other... well,
> I guess it's not too bad.  I'm not sure what to put in its place.

Alternately, we could just explicitly use subtract plus store.

This is what the x86 backend is going to do anyway, and avoids
the issue of odd-sized subregs of floating point values.  As 
you note, would have caused register allocation problems anyway.


r~

Re: pre_inc/pre_dec/PUSH_ROUNDING inconsistency

[Jan Hubicka]

> On Mon, Jul 17, 2000 at 02:16:04PM -0700, Richard Henderson wrote:
> > Yes, the TFmode subreg is incorrect.  As for the other... well,
> > I guess it's not too bad.  I'm not sure what to put in its place.
> 
> Alternately, we could just explicitly use subtract plus store.
> 
> This is what the x86 backend is going to do anyway, and avoids
> the issue of odd-sized subregs of floating point values.  As 
> you note, would have caused register allocation problems anyway.
OK, this seems to be best solution I've found so far too.
Perhaps the register allocation problems can be handled by insn patterns
explicitly mentioning the subregs like:
(set (match_operand:DI "push_operand") (subreg:DI (match_operand:SI "general_operand") 0))

But this is kludge and not going to solve problems with memory references (I
believe that combine never construct paradoxical subregs to memory or not?)

The kludge would work - I was playing similar games in one never reviewed patc
hwith mulsihi insn pattern (since this pattern is currently always promoted to
mulhi pattern by combine) and expect the problems in genrecog you perhaps
remember fixing about year ago (the fix was one of my not very lucky patches
especially hard to review)...

Problem is that the explicit subtract plus store method don't fit closely
to the existing way pushes are genrated. Ordinaly mov?i expander is used with
the push_operand in the destination.
Perhaps we can define new "push?i" standard names and add necesary optabs
code to imitate them using push_operand mov?i when not present.
This would not require any changes to existing MD files.

Alternate way is to default into two instruction - the store and the stack
adjustments. This would allow machine description to omit the push paterns
for modes hardware don't implement. Note that for i386 case this should not
be uesable, since even for fp stores we want to use integer push instruction
to store memory, immediates and integer regs.  So perhaps the first approach
is easier to implement and better.

What do you think?
Honza
> 
> 
> r~

Re: pre_inc/pre_dec/PUSH_ROUNDING inconsistency

[Jan Hubicka]

Hi
It looks to me, that current POST_MODIFY patches are exactly what we
need for way out of the PUSH_ROUNDING troubles.  Does sound sensible
to you patch to emit_push_insn that will just use POST_MODIFY instead
of PRE_DEC in case the PUSH_ROUNDING != GET_MODE_SIZE?
Or should we switch from PRE_* to POST_MODIFY entirely for calls.c
code to avoid complication in code recognizing push (currently only
known to my is in combine_stack_adjustments)?

Honza

Re: pre_inc/pre_dec/PUSH_ROUNDING inconsistency

[Richard Henderson]

On Sat, Jul 29, 2000 at 12:17:00AM +0200, Jan Hubicka wrote:
> It looks to me, that current POST_MODIFY patches are exactly what we
> need for way out of the PUSH_ROUNDING troubles.  Does sound sensible
> to you patch to emit_push_insn that will just use POST_MODIFY instead
> of PRE_DEC in case the PUSH_ROUNDING != GET_MODE_SIZE?

Or rather PRE_MODIFY you mean?  *shrug*  Perhaps.  The only
issue I see is that PUSH_ROUNDING is actually lying about
what the machine is doing, and that of course is causing the
problems.


r~