Re: Bug? flow_find_cross_jump deletes USE insns ...

Re: Bug? flow_find_cross_jump deletes USE insns ...

[Ulrich Weigand]

Jan Hubicka wrote:

>As I tried previously, CALL_INSN_FUCNTION_USGE way is tricky.
>At least on i386, you don't need GOT when function is called indirectly,
>so I don't add it to the FUSAGE, but when the indirect call is changed
>to direct, I get wrong insn, as I can't check validity of FUSAGE in
>the pattern.
>It is better to use separate USE IMO.

I'm not sure I understand the problem, maybe it's i386 specific ...
Could you elaborate?  Was does 'indirect call is changed to direct' mean?

On s390, we use the same insn for indirect and direct calls (a 'direct
call' is basically:  load target from literal pool into register, then
do an indirect call via that register).  If that target is a PLT stub,
then we need to have the GOT register set, if not then not.

What I've done now is to add the GOT register to the
CALL_INSN_FUNCTION_USAGE when constructing the 'load PLT address from
literal pool into register; call via register' sequence in gen_call.
In case of an indirect call I don't add it.  This seems to work fine
as far as I can see.  Why do you think it shouldn't work?

If I were to use an explicit USE, I'd have to duplicate all the call
insns to have one form with USE and one without; I don't see what this
would help.


Mit freundlichen Gruessen / Best Regards

Ulrich Weigand

--
  Dr. Ulrich Weigand
  Linux for S/390 Design & Development
  IBM Deutschland Entwicklung GmbH, Schoenaicher Str. 220, 71032 Boeblingen
  Phone: +49-7031/16-3727   ---   Email: Ulrich.Weigand@de.ibm.com

Re: Bug? flow_find_cross_jump deletes USE insns ...

[Richard Henderson]

On Tue, Jan 15, 2002 at 08:42:26PM +0100, Ulrich Weigand wrote:
> I'm not sure I understand the problem, maybe it's i386 specific ...
> Could you elaborate?  Was does 'indirect call is changed to direct' mean?

	void (*fn) (void) = foo;
	(*fn)();

CSE should be able to change this to

	foo();

Dunno if this is possible on s390 due to the constant pooling.
But if it _can_ happen, then your call pattern suddenly needs
the gp when it didn't have it before.


r~

Re: Bug? flow_find_cross_jump deletes USE insns ...

[Jan Hubicka]

> 
> Jan Hubicka wrote:
> 
> >As I tried previously, CALL_INSN_FUCNTION_USGE way is tricky.
> >At least on i386, you don't need GOT when function is called indirectly,
> >so I don't add it to the FUSAGE, but when the indirect call is changed
> >to direct, I get wrong insn, as I can't check validity of FUSAGE in
> >the pattern.
> >It is better to use separate USE IMO.
> 
> I'm not sure I understand the problem, maybe it's i386 specific ...
> Could you elaborate?  Was does 'indirect call is changed to direct' mean?
> 
> On s390, we use the same insn for indirect and direct calls (a 'direct
> call' is basically:  load target from literal pool into register, then
> do an indirect call via that register).  If that target is a PLT stub,
> then we need to have the GOT register set, if not then not.

I see, then you need PLT for each libcall.  On the i386 if the call is direct
(function symbolic address is known at compilation time), it goes to PLT and
needs GOT, unless it is static function where IP relative addressing is used.
In case it is indirect, it do use address of final function, so it does not
need GOT.
> 
> What I've done now is to add the GOT register to the
> CALL_INSN_FUNCTION_USAGE when constructing the 'load PLT address from
> literal pool into register; call via register' sequence in gen_call.

That should work in your case.

Honza

Re: Bug? flow_find_cross_jump deletes USE insns ...

[Richard Henderson]

On Mon, Jan 21, 2002 at 02:08:26PM -0800, Geoff Keating wrote:
> If you don't have any calls, how can the function throw?

The comment isn't quite correct.  These are also needed for
debuging unwind.


r~

Re: Bug? flow_find_cross_jump deletes USE insns ...

[Geoff Keating]

Jakub Jelinek <jakub@redhat.com> writes:

> On Mon, Jan 14, 2002 at 12:56:22PM -0800, Richard Henderson wrote:
> > On Mon, Jan 14, 2002 at 04:56:53PM +0100, Ulrich Weigand wrote:
> > > The prolog on S/390 generates a USE insn for the GOT register,
> > > because the GOT pointer must be loaded even if it isn't used
> > > in the routine (because the PLT stubs rely on the register).
> > 
> > This isn't going to work.  Bare USE insns aren't meaningful
> > after reload.  You need to add the USE to the call insn.  You
> > can do this either inside the call pattern itself, or by adding
> > it to the CALL_INSN_FUNCTION_USAGE expr_list.
> 
> I have a similar problem on IA-64, unfortunately there is no call where to
> stick the USEs in.

If you don't have any calls, how can the function throw?

...
> where the use is trying to make sure the set before it is not deleted:
>           /* Even if we're not going to generate an epilogue, we still
>              need to save the register so that EH works.  */
>           if (! epilogue_p)
>             emit_insn (gen_rtx_USE (VOIDmode, alt_reg));

-- 
- Geoffrey Keating <geoffk@geoffk.org> <geoffk@redhat.com>

Re: Bug? flow_find_cross_jump deletes USE insns ...

[Ulrich Weigand]

Jan Hubicka wrote:

>I see, then you need PLT for each libcall.  On the i386 if the call is
direct
>(function symbolic address is known at compilation time), it goes to PLT
and
>needs GOT, unless it is static function where IP relative addressing is
used.
>In case it is indirect, it do use address of final function, so it does
not
>need GOT.

Oops, I just noticed I forgot CC's on this mail I sent to Richard,
which shows the call patterns we are using:

>Ah, I see.  This can't really happen on s390 because we are using
>different UNSPEC's to distiguish between the GOT slot containing the
>address of foo (as used for function pointer comparisons) and the PLT
>slot of foo (used for direct calls).
>
>The first will expand (assuming small GOT) to something like
>
> (set (reg <addr>)
>      (mem (plus (reg <got-pointer>)
>                 (const (unspec [(symbol_ref ("foo"))] 110)))))
>
> (call (mem (reg <addr>))
>
>while the second will expand to (basically)
>
> (set (reg <addr>)
>      (const (unspec [(symbol_ref ("foo"))] 113)))
>
> (call (mem (plus (reg <addr>) (reg <literal-pool-base>)))
>
>where the first set is actually a load from the literal pool.
>
>Unspec 110 denotes the offset from the base of the GOT to the
>GOT slot for the function, while unspec 113 denotes the offset
>from the current function's literal pool base to the PLT slot
>of the called function.

So we only really have one call instruction, which implements
'call an effective address'.  We are using this instruction in
different ways:

 - Non-PIC direct calls:
   Load target address (from literal pool) into any
   address register <addr>;
   call <addr>

 - PIC direct calls to static functions:
   Load 'target address' - 'literal pool base' (from pool) into <addr>;
   call <addr>+<literal-pool-base-reg>

 - PIC direct calls to extern functions:
   Load 'PLT stub address' - 'literal pool base' into <addr>
   call <addr>+<literal-pool-base-reg>

 - Indirect calls:
   Load target function pointer into <addr>
   call <addr>

If a function pointer is to be loaded, we have these options:

 - Load function pointer (non-PIC):
   Load target address from literal pool

 - Load function pointer to static function (PIC):
   Load 'target address' - 'literal pool base'; add literal-pool-base-reg.

 - Load function pointer to extern function (PIC with small GOT):
   Dereference <got-register> + <GOT slot offset for function> to
   load address from GOT.

 - Load function pointer to extern function (PIC with big GOT)
   Load <GOT slot offset> from literal pool into <addr>;
   Dereference <got-register> + <addr> to load address from GOT

This should be quite similar to what i386 does, we just have to
adapt the implementation a bit to the different instruction set.
(No PC-relative calls, no addresses as immediate constants.)


Mit freundlichen Gruessen / Best Regards

Ulrich Weigand

--
  Dr. Ulrich Weigand
  Linux for S/390 Design & Development
  IBM Deutschland Entwicklung GmbH, Schoenaicher Str. 220, 71032 Boeblingen
  Phone: +49-7031/16-3727   ---   Email: Ulrich.Weigand@de.ibm.com

Re: Bug? flow_find_cross_jump deletes USE insns ...

[Richard Henderson]

On Wed, Jan 16, 2002 at 11:41:03AM +0100, Jakub Jelinek wrote:
> What is the preferred way here?
> Use some dummy unspec insn which will contain all the USEs, or use some
> unspec insn in the sets (like
> (set (reg:DI 33 r35) (unspec:DI[(reg:DI 320 b0)]  FOO))
> ), or something else?

Another way is to leave the existing insn as-is and have

 (unspec:DI [(reg:DI r35)] FOO)

and make it type ignore.  That creates a generic "use"
for the two or three places in the prologue that need this.


r~

Re: Bug? flow_find_cross_jump deletes USE insns ...

[Richard Henderson]

On Wed, Jan 16, 2002 at 11:41:03AM +0100, Jakub Jelinek wrote:
> Use some dummy unspec insn which will contain all the USEs, or use some
> unspec insn in the sets (like
> (set (reg:DI 33 r35) (unspec:DI[(reg:DI 320 b0)]  FOO))
> ), or something else?

That works for me.


r~

Re: Bug? flow_find_cross_jump deletes USE insns ...

[Ulrich Weigand]

Richard Henderson wrote:
>On Tue, Jan 15, 2002 at 09:41:04PM +0100, Ulrich Weigand wrote:
> So CSE can never make the optimization you describe above.
>
>Ok, sounds like you're safe.

Great, thanks.  I'll check in the variant that uses
CALL_INSN_FUNCTION_USAGE.

>> (I don't think there would be much benefit of trying to add this
>> optimization on our platform, given that the two call sequences
>> are basically equivalent - one load from memory, one indirect call ...)
>
>Yes, but one uses lazy binding and the other doesn't.  So ideally
>you'd attempt to do this.  Whether it ocurrs often enough to make
>it worth your while is another story.

Well, the PLT stubs themselves are quite inefficient currently,
because they incur a large penalty due to address generation
interlocks (they are so short there is no way to alleviate that
by instruction scheduling).

Therefore I have even been thinking of doing it the other way
around and prefering indirect calls over PLT stub calls.  However,
you're right about lazy binding ...

In any case, I won't be attempting any such optimization in the 3.1
time frame; maybe at some later point.


Mit freundlichen Gruessen / Best Regards

Ulrich Weigand

--
  Dr. Ulrich Weigand
  Linux for S/390 Design & Development
  IBM Deutschland Entwicklung GmbH, Schoenaicher Str. 220, 71032 Boeblingen
  Phone: +49-7031/16-3727   ---   Email: Ulrich.Weigand@de.ibm.com

Re: Bug? flow_find_cross_jump deletes USE insns ...

[Jakub Jelinek]

On Mon, Jan 14, 2002 at 12:56:22PM -0800, Richard Henderson wrote:
> On Mon, Jan 14, 2002 at 04:56:53PM +0100, Ulrich Weigand wrote:
> > The prolog on S/390 generates a USE insn for the GOT register,
> > because the GOT pointer must be loaded even if it isn't used
> > in the routine (because the PLT stubs rely on the register).
> 
> This isn't going to work.  Bare USE insns aren't meaningful
> after reload.  You need to add the USE to the call insn.  You
> can do this either inside the call pattern itself, or by adding
> it to the CALL_INSN_FUNCTION_USAGE expr_list.

I have a similar problem on IA-64, unfortunately there is no call where to
stick the USEs in.
If during ia64_expand_epilogue exit block is not reachable (thus epilogue_p
is 0), then it generates code like:
(insn/f 160 126 162 (parallel[
            (set (reg:DI 34 r36)
                (unspec_volatile:DI[
                        (const_int 0 [0x0])
                    ]  0))
            (use (const_int 2 [0x2]))
            (use (const_int 3 [0x3]))
            (use (const_int 2 [0x2]))
            (use (const_int 0 [0x0]))
        ] ) -1 (nil)
    (nil))
(insn/f 162 160 163 (set (reg:DI 33 r35)
        (reg:DI 320 b0)) -1 (nil)
    (nil))
(insn 163 162 164 (use (reg:DI 33 r35)) -1 (nil)
    (nil))

where the use is trying to make sure the set before it is not deleted:
          /* Even if we're not going to generate an epilogue, we still
             need to save the register so that EH works.  */
          if (! epilogue_p)
            emit_insn (gen_rtx_USE (VOIDmode, alt_reg));
If bare USE insns aren't meaningful after reload (and this is after reload),
then something else needs to be done.
What is the preferred way here?
Use some dummy unspec insn which will contain all the USEs, or use some
unspec insn in the sets (like
(set (reg:DI 33 r35) (unspec:DI[(reg:DI 320 b0)]  FOO))
), or something else?

	Jakub

Re: Bug? flow_find_cross_jump deletes USE insns ...

[Jan Hubicka]

> On Mon, Jan 14, 2002 at 04:56:53PM +0100, Ulrich Weigand wrote:
> > The prolog on S/390 generates a USE insn for the GOT register,
> > because the GOT pointer must be loaded even if it isn't used
> > in the routine (because the PLT stubs rely on the register).
> 
> This isn't going to work.  Bare USE insns aren't meaningful
> after reload.  You need to add the USE to the call insn.  You
> can do this either inside the call pattern itself, or by adding
> it to the CALL_INSN_FUNCTION_USAGE expr_list.

As I tried previously, CALL_INSN_FUCNTION_USGE way is tricky.
At least on i386, you don't need GOT when function is called indirectly,
so I don't add it to the FUSAGE, but when the indirect call is changed
to direct, I get wrong insn, as I can't check validity of FUSAGE in
the pattern.
It is better to use separate USE IMO.

Honza
> 
> 
> r~

Re: Bug? flow_find_cross_jump deletes USE insns ...

[Ulrich Weigand]

Richard Henderson wrote:

>On Mon, Jan 14, 2002 at 04:56:53PM +0100, Ulrich Weigand wrote:
>> The prolog on S/390 generates a USE insn for the GOT register,
>> because the GOT pointer must be loaded even if it isn't used
>> in the routine (because the PLT stubs rely on the register).
>
>This isn't going to work.  Bare USE insns aren't meaningful
>after reload.  You need to add the USE to the call insn.  You
>can do this either inside the call pattern itself, or by adding
>it to the CALL_INSN_FUNCTION_USAGE expr_list.

OK, I'll do it that way.

Thanks,
Ulrich

--
  Dr. Ulrich Weigand
  Linux for S/390 Design & Development
  IBM Deutschland Entwicklung GmbH, Schoenaicher Str. 220, 71032 Boeblingen
  Phone: +49-7031/16-3727   ---   Email: Ulrich.Weigand@de.ibm.com

Re: Bug? flow_find_cross_jump deletes USE insns ...

[Jan Hubicka]

> Hello,
> 
> a couple of glibc routines are miscompiled on S/390 because
> the cross-jump optimizations remove USE insns after reload.
> 
> The prolog on S/390 generates a USE insn for the GOT register,
> because the GOT pointer must be loaded even if it isn't used
> in the routine (because the PLT stubs rely on the register).

Why exactly do you need the USE instruction? i386 does use similar
scheme (it needs PLT for function calls) and what it does is to
emit explicit USE into each such call.
Alternative is just to maitain flag during code generation whetehr
GOT is needed or not.

It is bit dificult to keep those USE/CLOBBER instructions meaningfull
during compiler and it would be nice to avoid them.

Honza
> 
> This works fine normally, however it can happen that the
> cross-jump optimization pass decides to delete the very first
> insn after the prolog.  If this occurs, there is code in
> flow_find_cross_jump that, as the comment says, tries to
> 'include preceding notes and labels in the cross-junmp'.
> 
> However, it uses the 'active_insn_p' routine to check which preceding
> insns to include.  After reload, this predicate returns false
> not only for notes and labels, but also for USE and CLOBBER insns.
> 
> This means that the GOT register USE insn is killed, and later
> passes then decide to remove the GOT-setting insns as well ...
> 
> Is the use of a USE insn for this purpose (after reload) not
> supported after all?  Or is this a bug in cross-jump optimization?
> 
> 
> Mit freundlichen Gruessen / Best Regards
> 
> Ulrich Weigand
> 
> --
>   Dr. Ulrich Weigand
>   Linux for S/390 Design & Development
>   IBM Deutschland Entwicklung GmbH, Schoenaicher Str. 220, 71032 Boeblingen
>   Phone: +49-7031/16-3727   ---   Email: Ulrich.Weigand@de.ibm.com

Re: Bug? flow_find_cross_jump deletes USE insns ...

[Richard Henderson]

On Mon, Jan 14, 2002 at 04:56:53PM +0100, Ulrich Weigand wrote:
> The prolog on S/390 generates a USE insn for the GOT register,
> because the GOT pointer must be loaded even if it isn't used
> in the routine (because the PLT stubs rely on the register).

This isn't going to work.  Bare USE insns aren't meaningful
after reload.  You need to add the USE to the call insn.  You
can do this either inside the call pattern itself, or by adding
it to the CALL_INSN_FUNCTION_USAGE expr_list.


r~

Bug? flow_find_cross_jump deletes USE insns ...

[Ulrich Weigand]

Hello,

a couple of glibc routines are miscompiled on S/390 because
the cross-jump optimizations remove USE insns after reload.

The prolog on S/390 generates a USE insn for the GOT register,
because the GOT pointer must be loaded even if it isn't used
in the routine (because the PLT stubs rely on the register).

This works fine normally, however it can happen that the
cross-jump optimization pass decides to delete the very first
insn after the prolog.  If this occurs, there is code in
flow_find_cross_jump that, as the comment says, tries to
'include preceding notes and labels in the cross-junmp'.

However, it uses the 'active_insn_p' routine to check which preceding
insns to include.  After reload, this predicate returns false
not only for notes and labels, but also for USE and CLOBBER insns.

This means that the GOT register USE insn is killed, and later
passes then decide to remove the GOT-setting insns as well ...

Is the use of a USE insn for this purpose (after reload) not
supported after all?  Or is this a bug in cross-jump optimization?


Mit freundlichen Gruessen / Best Regards

Ulrich Weigand

--
  Dr. Ulrich Weigand
  Linux for S/390 Design & Development
  IBM Deutschland Entwicklung GmbH, Schoenaicher Str. 220, 71032 Boeblingen
  Phone: +49-7031/16-3727   ---   Email: Ulrich.Weigand@de.ibm.com