From: Richard Biener <richard.guenther@gmail.com>
To: "Bin.Cheng" <amker.cheng@gmail.com>
Cc: Bin Cheng <bin.cheng@arm.com>, GCC Patches <gcc-patches@gcc.gnu.org>
Subject: Re: [PATCH PR67921]Use sizetype for CHREC_RIGHT when building pointer type CHREC
Date: Wed, 21 Oct 2015 12:45:00 -0000 [thread overview]
Message-ID: <CAFiYyc3nKtG6b9KyMd43gGrqMNpi73OG4Wx5Lesdtk84Yjbmtg@mail.gmail.com> (raw)
In-Reply-To: <CAHFci297i-gNwpKWZJ=cey479CnGv2Cza+J4fF41u_oviRRxSg@mail.gmail.com>
On Wed, Oct 21, 2015 at 11:39 AM, Bin.Cheng <amker.cheng@gmail.com> wrote:
> On Wed, Oct 21, 2015 at 5:15 PM, Richard Biener
> <richard.guenther@gmail.com> wrote:
>> On Wed, Oct 21, 2015 at 6:46 AM, Bin Cheng <bin.cheng@arm.com> wrote:
>>> Hi,
>>> As analyzed in PR67921, I think the issue is caused by fold_binary_loc which
>>> folds:
>>> 4 - (sizetype) &c - (sizetype) ((int *) p1_8(D) + ((sizetype) a_23 * 24 +
>>> 4))
>>> into below form:
>>> ((sizetype) -((int *) p1_8(D) + ((sizetype) a_23 * 24 + 4)) - (sizetype)
>>> &c) + 4
>>>
>>> Look the minus sizetype expression is folded as negative pointer expression,
>>> which seems incorrect. Apart from this, The direct reason of this ICE is in
>>> CHREC because of an overlook. In general CHREC supports NEGATE_EXPR for
>>> CHREC, the only problem is it uses pointer type for CHREC_RIGHT, rather than
>>> sizetype, when building pointer type CHREC.
>>>
>>> This simple patch fixes the ICE issue. Bootstrap and test on x86 & x86_64.
>>>
>>> Is it OK?
>>
>> Hmm, I think not - we shouldn't ever get pointer typed
>> multiplications. Did you track
>> down which is the bogus fold transform (I agree the result above is
>> bogus)? It's
>> probably related to STRIP_NOPS stripping sizetype conversions from pointers
>> so we might get split_tree to build such negate. Note that split_tree strips
>> (sign!) nops itself and thus should probably simply receive op0 and op1 instead
>> of arg0 and arg1.
> Yes, I was going to send similar patch for fold stuff. Just thought
> it might be useful to support POINTER chrec in *_multiply. I will
> drop this and let you test yours.
The patch regresses
Running target unix/
FAIL: gcc.dg/pr52578.c scan-tree-dump-times original "return 2;" 2
FAIL: gcc.dg/strict-overflow-5.c scan-tree-dump optimized "return 3"
FAIL: gcc.dg/tree-ssa/loop-15.c scan-tree-dump-times optimized "\\\\+" 0
FAIL: gcc.dg/tree-ssa/loop-15.c scan-tree-dump-times optimized "n_. \\\\* n_." 1
FAIL: gcc.dg/vect/vect-align-3.c -flto -ffat-lto-objects scan-tree-dump-not vec
t "vect_do_peeling_for_loop_bound"
FAIL: gcc.dg/vect/vect-align-3.c scan-tree-dump-not vect "vect_do_peeling_for_lo
op_bound"
and for -m32
FAIL: gcc.target/i386/pr67985-3.c scan-assembler movd[ \\t]%xmm[0-7], %eax
FAIL: gcc.target/i386/pr67985-3.c scan-assembler mulss
as well. So it requires some more complicated handling. It might need to
take the negation code in split-tree to be performed only implicitely during
associate_trees (then on the correct type). But that requires (quite) some
refactoring. I suggest a struct tree_with_negate { tree t; bool negate_p; }
to pass this along.
Not for me at this point (looks I have quite a few patches to review ;))
Richard.
> Thanks,
> bin
>>
>> I'm testing
>>
>> @@ -9505,8 +9523,8 @@ fold_binary_loc (location_t loc,
>> then the result with variables. This increases the chances of
>> literals being recombined later and of generating relocatable
>> expressions for the sum of a constant and literal. */
>> - var0 = split_tree (arg0, code, &con0, &lit0, &minus_lit0, 0);
>> - var1 = split_tree (arg1, code, &con1, &lit1, &minus_lit1,
>> + var0 = split_tree (op0, code, &con0, &lit0, &minus_lit0, 0);
>> + var1 = split_tree (op1, code, &con1, &lit1, &minus_lit1,
>> code == MINUS_EXPR);
>>
>> /* Recombine MINUS_EXPR operands by using PLUS_EXPR. */
>>
>> which fixes the testcase for me.
>>
>> Richard.
>>
>>> Note, I do think the associate logic in fold_binary_loc needs fix, but that
>>> should be another patch.
>>>
>>>
>>> 2015-10-20 Bin Cheng <bin.cheng@arm.com>
>>>
>>> PR tree-optimization/67921
>>> * tree-chrec.c (chrec_fold_multiply): Use sizetype for CHREC_RIGHT
>>> if
>>> type is pointer type.
>>>
>>> 2015-10-20 Bin Cheng <bin.cheng@arm.com>
>>>
>>> PR tree-optimization/67921
>>> * gcc.dg/ubsan/pr67921.c: New test.
prev parent reply other threads:[~2015-10-21 12:12 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-10-21 4:58 Bin Cheng
2015-10-21 9:16 ` Richard Biener
2015-10-21 9:43 ` Bin.Cheng
2015-10-21 12:45 ` Richard Biener [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAFiYyc3nKtG6b9KyMd43gGrqMNpi73OG4Wx5Lesdtk84Yjbmtg@mail.gmail.com \
--to=richard.guenther@gmail.com \
--cc=amker.cheng@gmail.com \
--cc=bin.cheng@arm.com \
--cc=gcc-patches@gcc.gnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).