This is the mail archive of the
mailing list for the GCC project.
Re: Potential fix for rdar://4658012
- From: kenner at vlsi1 dot ultra dot nyu dot edu (Richard Kenner)
- To: jconner at apple dot com
- Cc: gcc-patches at gcc dot gnu dot org, jason at redhat dot com, richard dot guenther at gmail dot com
- Date: Sat, 09 Sep 2006 08:45:52 EDT
- Subject: Re: Potential fix for rdar://4658012
- References: <44EF948E.email@example.com> <firstname.lastname@example.org> <10608260940.AA19284@vlsi1.ultra.nyu.edu> <email@example.com> <10608261003.AA19387@vlsi1.ultra.nyu.edu> <44F32FAC.firstname.lastname@example.org> <10608312235.AA10712@vlsi1.ultra.nyu.edu> <email@example.com>
> Yes, this makes perfect sense. I can envision a tree where removing
> this code would create an invalid reuse of the stack slot. However, we
> are doing two things to preserve this temp -- marking it with
> "addr_taken" and creating it with the "keep" bit set. The former says
> "promote this temp one level up", and the latter "keep this temp around
> for the lifetime of the current block". This still seems redundant. I
> think marking the temp as addr_taken is sufficient to handle the case
> where the address is used as the input to a function call (see attached
Indeed it should be, but I wonder what the history of that code is.
The "addr_taken" line was added to fix that specific bug, so I wonder
if "keep" was also set at that time. I suspect not, but I'm not sure it's
worth doing that much archeology.
> Note that since Jason's recent change to the frontend here:
> This change would be sufficient to address the remaining issues in
> pr25505. I'd like to hear your opinion of this, though, before
> submitting it for approval.
Does "it" mean what's below (i.e., removing "keep")? If so, my feeling would
be to agree with you that it seems redundant and hence not needed, but I'd
also fear introducing bugs here. At a minimum, I'd suggest running a full
test on Sparc (which is where these bugs will show up most often due to its
calling convention) if you can.
> To help address your concerns about fixing the regression closest to its
> point of change, I've also been looking into how this was handled in
> 3.4. In that version of the compiler, a TARGET_EXPR was surviving all
> the way until the expander, where the function expand_expr_real was
> recognizing TARGET_EXPRs and generating a reusable temp. Unfortunately,
> I don't see an obvious correlation in 4.x, as the TARGET_EXPR never
> reaches the expander, since gimplification has already converted it into
> a CALL_EXPR.
So what you're basically saying is that it's the act of gimplification which
is adding the extra storage requirement?