diff mbox

Mark variables addressable if they are copied using libcall in RTL expander

Message ID BANLkTimie4iac=V5bqeGcRp9xUbU1-3ak-hwSXfYtpJxToQUig@mail.gmail.com
State New
Headers show

Commit Message

Easwaran Raman June 22, 2011, 7:33 p.m. UTC
On Wed, Jun 22, 2011 at 7:13 AM, Eric Botcazou <ebotcazou@adacore.com> wrote:
>> I fear this isn't enough considering pass-by-value aggregates that
>> are callee copied.
>
> It's indeed not sufficient for arguments passed by reference but callee-copied.
>
> This is PR target/49454.  For gcc.c-torture/execute/20000717-1.c:
>
> typedef struct trio { int a, b, c; } trio;
>
> int
> foo (trio t, int i)
> {
>  return bar (i, t);
> }
>
> yiedls in the .optimized dump:
>
> foo (struct trio t, int i)
> {
>  int D.1968;
>  struct trio t.0;
>
> <bb 2>:
>  t.0 = t;
>  D.1968_2 = bar (i_1(D), t.0);
>  return D.1968_2;
> }
>
> and the aggregate copy is elided by DSE because t.0 isn't may_be_aliased.  This
> seems to be a pre-existing bug though: its address is passed to bar in RTL.
>
> --
> Eric Botcazou
>

Is the following patch a reasonable fix for this case?  I assume I
should add similar code inside emit_library_call_value_1.

-Easwaran

Comments

Eric Botcazou June 23, 2011, 8 a.m. UTC | #1
> Is the following patch a reasonable fix for this case?

The lines should be moved to within the first branch of the subsequent "if". 
They aren't needed if the second branch is taken because, in this case, we're 
back to the usual caller-copied scheme where we pass the address of the copy.

> I assume I should add similar code inside emit_library_call_value_1.

Yes, we need the same treatment for 'val' in the MEM_P (val) && !must_copy case 
as the one applied in emit_block_move_hints.


But these problems show that there is a slight discrepancy between what dse.c 
really needs (is the address of the variable taken?) and what may_be_aliased 
answers (might the variable be indirectly modified?).  Another viewpoint is to 
say that there is a slight discrepancy between Tree and RTL level when it comes 
to the address-taken property.  Not clear what to do about it so I think that 
we should try this kludgy way and see how it fares.
Richard Biener June 23, 2011, 10:05 a.m. UTC | #2
On Thu, Jun 23, 2011 at 10:00 AM, Eric Botcazou <ebotcazou@adacore.com> wrote:
>> Is the following patch a reasonable fix for this case?
>
> The lines should be moved to within the first branch of the subsequent "if".
> They aren't needed if the second branch is taken because, in this case, we're
> back to the usual caller-copied scheme where we pass the address of the copy.
>
>> I assume I should add similar code inside emit_library_call_value_1.
>
> Yes, we need the same treatment for 'val' in the MEM_P (val) && !must_copy case
> as the one applied in emit_block_move_hints.
>
>
> But these problems show that there is a slight discrepancy between what dse.c
> really needs (is the address of the variable taken?) and what may_be_aliased
> answers (might the variable be indirectly modified?).  Another viewpoint is to
> say that there is a slight discrepancy between Tree and RTL level when it comes
> to the address-taken property.  Not clear what to do about it so I think that
> we should try this kludgy way and see how it fares.

Yeah, I agree.  Unless we want to really do alias analysis on RTL (and not just
export what the tree level has in some way) let's go forward with this.

So, what's the patch(es) that need approval now?

Thanks,
Richard.

> --
> Eric Botcazou
>
Eric Botcazou June 23, 2011, 10:22 a.m. UTC | #3
> So, what's the patch(es) that need approval now?

Original expr.c patch for PR rtl-optimization/49429 + adjusted and augmented 
calls.c patch for PR target/49454.  Everything is in this thread.

Easwaran, would you mind posting a consolidated patch?
diff mbox

Patch

--- gcc/calls.c	(revision 175081)
+++ gcc/calls.c	(working copy)
@@ -1073,6 +1073,8 @@  initialize_argument_information (int num_actuals A
 	  callee_copies
 	    = reference_callee_copied (args_so_far, TYPE_MODE (type),
 				       type, argpos < n_named_args);
+          if (callee_copies)
+            mark_addressable (args[i].tree_value);

 	  /* If we're compiling a thunk, pass through invisible references
 	     instead of making a copy.  */