* [PATCH PR67921]Use sizetype for CHREC_RIGHT when building pointer type CHREC
@ 2015-10-21 4:58 Bin Cheng
2015-10-21 9:16 ` Richard Biener
0 siblings, 1 reply; 4+ messages in thread
From: Bin Cheng @ 2015-10-21 4:58 UTC (permalink / raw)
To: gcc-patches
[-- Attachment #1: Type: text/plain, Size: 1119 bytes --]
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?
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.
[-- Attachment #2: pr67921-20151020.txt --]
[-- Type: text/plain, Size: 1467 bytes --]
diff --git a/gcc/tree-chrec.c b/gcc/tree-chrec.c
index 649c9fe..ef7b70b 100644
--- a/gcc/tree-chrec.c
+++ b/gcc/tree-chrec.c
@@ -436,7 +436,8 @@ chrec_fold_multiply (tree type,
return build_polynomial_chrec
(CHREC_VARIABLE (op0),
chrec_fold_multiply (type, CHREC_LEFT (op0), op1),
- chrec_fold_multiply (type, CHREC_RIGHT (op0), op1));
+ chrec_fold_multiply (POINTER_TYPE_P (type) ? sizetype : type,
+ CHREC_RIGHT (op0), op1));
}
CASE_CONVERT:
@@ -459,7 +460,8 @@ chrec_fold_multiply (tree type,
return build_polynomial_chrec
(CHREC_VARIABLE (op1),
chrec_fold_multiply (type, CHREC_LEFT (op1), op0),
- chrec_fold_multiply (type, CHREC_RIGHT (op1), op0));
+ chrec_fold_multiply (POINTER_TYPE_P (type) ? sizetype : type,
+ CHREC_RIGHT (op1), op0));
CASE_CONVERT:
if (tree_contains_chrecs (op1, NULL))
diff --git a/gcc/testsuite/gcc.dg/ubsan/pr67921.c b/gcc/testsuite/gcc.dg/ubsan/pr67921.c
new file mode 100644
index 0000000..5e7d707
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/ubsan/pr67921.c
@@ -0,0 +1,22 @@
+/* { dg-do compile } */
+/* { dg-options "-fsanitize=undefined" } */
+
+typedef struct {
+ int a;
+ int arr[][6];
+}st;
+
+void bar (int);
+void foo (st *p)
+{
+ int a;
+ for (; a < 2; a++)
+ for (; p->a;)
+ {
+ int *b = p->arr[a];
+ int c[66];
+ int j = 0;
+ for (; j < 56; j++)
+ bar (b[j] - c[j]);
+ }
+}
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH PR67921]Use sizetype for CHREC_RIGHT when building pointer type CHREC
2015-10-21 4:58 [PATCH PR67921]Use sizetype for CHREC_RIGHT when building pointer type CHREC Bin Cheng
@ 2015-10-21 9:16 ` Richard Biener
2015-10-21 9:43 ` Bin.Cheng
0 siblings, 1 reply; 4+ messages in thread
From: Richard Biener @ 2015-10-21 9:16 UTC (permalink / raw)
To: Bin Cheng; +Cc: GCC Patches
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.
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.
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH PR67921]Use sizetype for CHREC_RIGHT when building pointer type CHREC
2015-10-21 9:16 ` Richard Biener
@ 2015-10-21 9:43 ` Bin.Cheng
2015-10-21 12:45 ` Richard Biener
0 siblings, 1 reply; 4+ messages in thread
From: Bin.Cheng @ 2015-10-21 9:43 UTC (permalink / raw)
To: Richard Biener; +Cc: Bin Cheng, GCC Patches
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.
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.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH PR67921]Use sizetype for CHREC_RIGHT when building pointer type CHREC
2015-10-21 9:43 ` Bin.Cheng
@ 2015-10-21 12:45 ` Richard Biener
0 siblings, 0 replies; 4+ messages in thread
From: Richard Biener @ 2015-10-21 12:45 UTC (permalink / raw)
To: Bin.Cheng; +Cc: Bin Cheng, GCC Patches
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.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2015-10-21 12:12 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-10-21 4:58 [PATCH PR67921]Use sizetype for CHREC_RIGHT when building pointer type CHREC Bin Cheng
2015-10-21 9:16 ` Richard Biener
2015-10-21 9:43 ` Bin.Cheng
2015-10-21 12:45 ` Richard Biener
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).