This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH, middle-end]: Fix PR 43057, [LTO] fold check: original tree changed by fold


On Sun, Nov 21, 2010 at 10:13 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Sun, Nov 21, 2010 at 09:48:54PM +0100, Richard Guenther wrote:
>> > Actually, --enable-checking=all build broke in fold-const.c, line
>> > 13361 (trying to build libstdc++/src/debug.cc) in the same call to
>> > pedantic_non_lvalue_loc. This points to the problem in the called
>> > function itself.
>> >
>> > Attached patch unshares the expression in the pedantic_non_lvalue_loc
>> > itself. We shouldn't change the original operand by setting the
>> > location.
>> >
>> > 2010-11-21 ?Uros Bizjak ?<ubizjak@gmail.com>
>> >
>> > ? ? ? ?PR middle-end/43057
>> > ? ? ? ?* fold-const.c (pedantic_non_lvalue_loc): Unshare x before
>> > ? ? ? ?setting location.
>> >
>> > OK for mainline and 4.5?
>>
>> Ok.
>
> I don't think this is desirable.
> 1) unshare_expr won't unshare SAVE_EXPR/TARGET_EXPR/BIND_EXPR
> 2) you unshare even when there is no reason for it (!CAN_HAVE_LOCATION_P
> (x) or when it already has the desired locus)
> 3) you don't need to do a deep unshare just to set location
>
> So, I think it would be much better to do something like:
> if (CAN_HAVE_LOCATION_P (x)
> ? ?&& EXPR_LOCATION (x) != loc
> ? ?&& TREE_CODE (x) != SAVE_EXPR
> ? ?&& TREE_CODE (x) != TARGET_EXPR
> ? ?&& TREE_CODE (x) != BIND_EXPR)
> ?{
> ? ?x = copy_node (x);
> ? ?SET_EXPR_LOCATION (x, loc);
> ?}
>
> Also, as I said in the PR, this isn't the only place in fold-const.c that
> needs fixing for --enable-checking=fold, there are more than 10 similar
> spots. ?And buildN_loc should be introduced and used where possible.

Your proposed approach also works (and fixes bootstrap with
--enable-checking=all, too). If there are no objections from Richi, I
propose to commit this change to mainline (since it enables bootstrap
with --enable-checking=fold) and fix all fold fails from the testsuite
separately.

Thanks,
Uros.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]