[patch] beat location sense into fold (PR/40435)

Richard Guenther richard.guenther@gmail.com
Thu Jul 9 12:49:00 GMT 2009


On Thu, Jul 9, 2009 at 2:30 PM, Aldy Hernandez<aldyh@redhat.com> wrote:
> Hi folks.
>
> Sorry it took so long, but this was far more work than I had
> anticipated.
>
> The following incredibly boring yet painfully tedious patch adds
> location information to fold_{unary,binary,etc} and fold_build[123]--
> basically all of fold-const.c and supporting machinery.
>
> [After seeing the magnitude of my changes, I chickened out of converting
> all uses of fold_convert() and size_binop().  Instead, I made location
> aware versions (fold_convert_loc() and size_binop_op()), and defined
> macros for fold_convert() and size_binop() that just pass
> input_location.  So only the relevant calls to fold_convert/size_binop
> were changed.]
>
> The reason for this patch was to fix PR/40435 which I caused after the
> diagnostics merge.  The problem in the PR was that warnings in an
> inlined function were being attributed to the caller, not the callee,
> because during tree inlining we were getting the location from the
> caller.
>
> This patch fixes gcc.dg/Wshadow-3.c and gcc.dg/pr36902.c without
> introducing any regressions.
>
> It has been tested on x86-64 Linux with all languages enabled, including
> obj-c++ and Ada.  I have also built cc1 for alpha, ia64, mep, mips, pa,
> rs6000, sh, sparc, stormy16, and xtensa.
>
> The patch is over a megabyte long, and I'm sure no one wants to see it,
> let alone have it in their inbox.  It is available in the link below for
> review.  I can post it on request.
>
> http://quesejoda.com/patches/fold-locations-patch
>
> OK for mainline?

When I see

Index: tree-ssa-loop-niter.c
===================================================================
--- tree-ssa-loop-niter.c       (revision 149369)
+++ tree-ssa-loop-niter.c       (working copy)
...
-                       fold_build1 (NEGATE_EXPR, type, iv->step));
-      c = fold_build2 (MINUS_EXPR, niter_type,
+                       fold_build1 (UNKNOWN_LOCATION,
+                                    NEGATE_EXPR, type, iv->step));
+      c = fold_build2 (UNKNOWN_LOCATION,
+                      MINUS_EXPR, niter_type,
                       fold_convert (niter_type, iv->base),
...

then I think you should have chickened out for all fold_* stuff as well
and created fold_buildN_loc instead.  Changes like the above only
make the code less readable for stuff that will never have a location
anyway.

And, of course, it makes my undefined-overflow branch a complete
conflict.  Likely anyway, but ... :/

So - I'm sorry, but I ask you to add the fold_buildN_loc variants
instead and only use it where it makes sense.  Just for the
reason that the variant with the location doesn't make sense
everywhere.  Not so much for my own convenience.

Thanks,
Richard.



More information about the Gcc-patches mailing list