Not using DECL_VALUE_EXPR for callee-copied parameters?

Not using DECL_VALUE_EXPR for callee-copied parameters?

[Martin Jambor]

Hi,

we sometimes currently mess up when compiling callee-copied parameters
on architectures that use it (so, as far as I know, on hppa).  The
manifestation of the issue is e.g. PR 40464 but I believe there maight
be others.

The real problem is that even though a comment in tree.h says that
declarations with a DECL_VALUE_EXPR "once this field has been set, the
decl itself may not legitimately appear in the function," they do
because gimplify_parameters() in function.c generates an explicit copy
statements.  This then has unintended wrong consequences when such a
declaration is later fed to the gimplifier again (for example through
force_gimple_operand) because they are replaced with their value_expr.

For reference, this is PR 41250.

Because DECL_VALUE_EXPRs affect debug info I attempted to simply not
doing the replacements in gimplifier but ran into all sorts of asserts
in expand and later even to execution failures which altogether looked
too overwhelming and so I gave up.  At the moment I intend to simply
stop making these callee-copied parameters value_expr ones since the
convention is implemented explicitly - this amounts to the two line
patch below.  Obviously, this will impair debug info but I hope that
for this purpose we can represent the relationship in another way.
Specifically, I was hoping to be able to use BLOCK_NONLOCALIZED_VARS
in the form in which they are in pretty-ipa where they essentially map
no longer existing decls to expressions evaluating their values.
Honza, do you think they will work also for an existing PARM_DECL?

I have run the testsuite on hppa with the following patch and I
successfully bootstrapped and tested it on x86_64.  Unless someone
objects, I will bootstrap it on hppa and commit it in a week or two
along with Richi's patch verifying such decls don't leak to the IL
(and then try to use the mechanism above to keep the debug info).

Thanks,

Martin


2009-09-11  Martin Jambor  <mjambor@suse.cz>

	* function.c (gimplify_parameters): Do not set a value-expr on
	callee-copied parm decls.


Index: small/gcc/function.c
===================================================================
--- small.orig/gcc/function.c
+++ small/gcc/function.c
@@ -3425,9 +3425,6 @@ gimplify_parameters (void)
 		}
 
 	      gimplify_assign (local, parm, &stmts);
-
-	      SET_DECL_VALUE_EXPR (parm, local);
-	      DECL_HAS_VALUE_EXPR_P (parm) = 1;
 	    }
 	}
     }


PS:  If you are interested what other purposes value_exprs are used
to, here is a list of places where SET_DECL_VALUE_EXPR is called, it
is such a weird thing in gimple that it might be a worthy goal to
replace them all with BLOCK_NONLOCALIZED_VARS:

*** function.c:
gimplify_parameters[3429]      SET_DECL_VALUE_EXPR (parm, local);

*** gimplify.c:
gimplify_vla_decl[1320]        SET_DECL_VALUE_EXPR (decl, t);
gimplify_var_or_parm_decl[1900] SET_DECL_VALUE_EXPR (copy, unshare_expr (value_expr));

*** omp-low.c:
fixup_remapped_decl[1029]      SET_DECL_VALUE_EXPR (new_decl, ve);
lower_rec_input_clauses[2382]  SET_DECL_VALUE_EXPR (new_var, x);
lower_rec_input_clauses[2438]  SET_DECL_VALUE_EXPR (new_var, x);
lower_rec_input_clauses[2465]  SET_DECL_VALUE_EXPR (placeholder, x);
lower_reduction_clauses[2672]  SET_DECL_VALUE_EXPR (placeholder, ref);

*** tree-cfg.c:
replace_block_vars_by_duplicates[6183] SET_DECL_VALUE_EXPR (t, DECL_VALUE_EXPR (*tp));

*** tree-nested.c:
get_nonlocal_debug_decl[839]   SET_DECL_VALUE_EXPR (new_decl, x);
get_local_debug_decl[1324]     SET_DECL_VALUE_EXPR (new_decl, x);
remap_vla_decls[2159]          SET_DECL_VALUE_EXPR (var, val);

*** tree.c:
copy_node_stat[955]            SET_DECL_VALUE_EXPR (t, DECL_VALUE_EXPR (node));

Re: Not using DECL_VALUE_EXPR for callee-copied parameters?

[Ian Lance Taylor]

Martin Jambor <mjambor@suse.cz> writes:

> I have run the testsuite on hppa with the following patch and I
> successfully bootstrapped and tested it on x86_64.  Unless someone
> objects, I will bootstrap it on hppa and commit it in a week or two
> along with Richi's patch verifying such decls don't leak to the IL
> (and then try to use the mechanism above to keep the debug info).

Without meaning to criticize your investigation, I object on procedural
grounds--some maintainer must approve such a patch to mainline.  I don't
think it qualifies as obvious.

Ian

Re: Not using DECL_VALUE_EXPR for callee-copied parameters?

[Martin Jambor]

On Fri, Sep 11, 2009 at 06:39:10PM -0700, Ian Lance Taylor wrote:
> Martin Jambor <mjambor@suse.cz> writes:
> 
> > I have run the testsuite on hppa with the following patch and I
> > successfully bootstrapped and tested it on x86_64.  Unless someone
> > objects, I will bootstrap it on hppa and commit it in a week or two
> > along with Richi's patch verifying such decls don't leak to the IL
> > (and then try to use the mechanism above to keep the debug info).
> 
> Without meaning to criticize your investigation, I object on procedural
> grounds--some maintainer must approve such a patch to mainline.  I don't
> think it qualifies as obvious.
> 

I was apparently too tired when writing this, I obviously meant to
propose to commit this in the list and hopefully get an approval.  I
just wanted to find out whether people would object to this first as
bootstrapping and testing on hppa is tedious and there will be all
sorts of weird problems making the debug info work again.

Having said that, if anybody has problems with this patch going in,
please speak out now rather than later.

Sorry for the confusion,

Martin

Re: Not using DECL_VALUE_EXPR for callee-copied parameters?

[John David Anglin]

Hi Martin,

> Having said that, if anybody has problems with this patch going in,
> please speak out now rather than later.

I very much appreciate the work you have done on this problem.  However,
the patch doesn't work as expected.  I get multiple comparison failures
with this change on hppa-unknown-linux-gnu and hppa2.0w-hp-hpux11.11.

Dave
-- 
J. David Anglin                                  dave.anglin@nrc-cnrc.gc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6602)

Re: Not using DECL_VALUE_EXPR for callee-copied parameters?

[Richard Guenther]

On Mon, Sep 14, 2009 at 3:52 AM, John David Anglin
<dave@hiauly1.hia.nrc.ca> wrote:
> Hi Martin,
>
>> Having said that, if anybody has problems with this patch going in,
>> please speak out now rather than later.
>
> I very much appreciate the work you have done on this problem.  However,
> the patch doesn't work as expected.  I get multiple comparison failures
> with this change on hppa-unknown-linux-gnu and hppa2.0w-hp-hpux11.11.

While in long-term it might be good to make DECL_VALUE_EXPR a
debuginfo only thing in short term the better approach would be to
have a special gimplifier state that turns on DECL_VALUE_EXPR
treatment (which would be off by default and thus during
force_gimple_operand).

The documentation of DECL_VALUE_EXPR should be adjusted
accordingly of course.

Richard.

> Dave
> --
> J. David Anglin                                  dave.anglin@nrc-cnrc.gc.ca
> National Research Council of Canada              (613) 990-0752 (FAX: 952-6602)
>