From: "Bin.Cheng" <amker.cheng@gmail.com>
To: Richard Biener <richard.guenther@gmail.com>
Cc: Bin Cheng <Bin.Cheng@arm.com>,
"gcc-patches@gcc.gnu.org" <gcc-patches@gcc.gnu.org>,
nd <nd@arm.com>
Subject: Re: [PATCH PR80153]Always generate folded type conversion in tree-affine
Date: Wed, 29 Mar 2017 15:32:00 -0000 [thread overview]
Message-ID: <CAHFci2-8Rn_oQ0UpcAQr2ByAYX0KVZRVV_6irffK8BpXifSj1Q@mail.gmail.com> (raw)
In-Reply-To: <CAFiYyc0W+8OonAwMrCNJR8VjRaZP9Rso7mcLggcYTQhrRT0crQ@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 3979 bytes --]
On Tue, Mar 28, 2017 at 1:34 PM, Richard Biener
<richard.guenther@gmail.com> wrote:
> On Tue, Mar 28, 2017 at 2:01 PM, Bin Cheng <Bin.Cheng@arm.com> wrote:
>> Hi,
>> This patch is to fix PR80153. As analyzed in the PR, root cause is tree_affine lacks
>> ability differentiating (unsigned)(ptr + offset) and (unsigned)ptr + (unsigned)offset,
>> even worse, it always returns the former expression in aff_combination_tree, which
>> is wrong if the original expression has the latter form. The patch resolves the issue
>> by always returning the latter form expression, i.e, always trying to generate folded
>> expression. Also as analyzed in comment, I think this change won't result in substantial
>> code gen difference.
>> I also need to adjust get_computation_aff for test case gcc.dg/tree-ssa/reassoc-19.c.
>> Well, I think the changed behavior is correct, but for case the original pointer candidate
>> is chosen, it should be unnecessary to compute in uutype. Also this adjustment only
>> generates (unsigned)(pointer + offset) which is generated by tree-affine.c.
>> Bootstrap and test on x86_64 and AArch64. Is it OK?
>
Thanks for reviewing.
> Hmm. What is the desired goal? To have all elts added have
> comb->type as type? Then
> the type passed to add_elt_to_tree is redundant with comb->type. It
> looks like it
> is always passed comb->type now.
Yes, except pointer type comb->type, elts are converted to comb->type
with this patch.
The redundant type is removed in updated patch.
>
> ISTR from past work in this area that it was important for pointer
> combinations to allow
> both pointer and sizetype elts at least.
Yes, It's still important to allow different types for pointer and
offset in pointer type comb.
I missed a pointer type check condition in the patch, fixed in updated patch.
>
> Your change is incomplete I think, for the scale == -1 and POINTER_TYPE_P case
> elt is sizetype now, not of pointer type. As said above, we are
> trying to maintain
> both pointer and sizetype elts with like:
>
> if (scale == 1)
> {
> if (!expr)
> {
> if (POINTER_TYPE_P (TREE_TYPE (elt)))
> return elt;
> else
> return fold_convert (type1, elt);
> }
>
> where your earilier fold to type would result in not all cases handled the same
> (depending whether scale was -1 for example).
IIUC, it doesn't matter. For comb->type being pointer type, the
behavior remains the same.
For comb->type being unsigned T, this elt is converted to ptr_offtype,
rather than unsigned T,
this doesn't matter because ptr_offtype and unsigned T are equal to
each other, otherwise
tree_to_aff_combination shouldn't distribute it as a single elt.
Anyway, this is addressed in updated patch by checking pointer
comb->type additionally.
BTW, I think "scale==-1" case is a simple heuristic differentiating
pointer_base and offset.
>
> Thus - shouldn't we simply drop the type argument (or rather the comb one?
> that wide_int_ext_for_comb looks weird given we get a widest_int as input
> and all the other wide_int_ext_for_comb calls around).
>
> And unconditionally convert to type, simplifying the rest of the code?
As said, for pointer type comb, we need to keep current behavior; for
other cases,
unconditionally convert to comb->type is the goal.
Bootstrap and test on x86_64 and AArch64. Is this version OK?
Thanks,
bin
2017-03-28 Bin Cheng <bin.cheng@arm.com>
PR tree-optimization/80153
* tree-affine.c (add_elt_to_tree): Remove parameter TYPE. Use type
of parameter COMB. Convert elt to type of COMB it COMB is not of
pointer type.
(aff_combination_to_tree): Update calls to add_elt_to_tree.
* tree-ssa-loop-ivopts.c (alloc_iv): Pass in consistent types.
(get_computation_aff): Use utype directly for original candidate.
gcc/testsuite/ChangeLog
2017-03-28 Bin Cheng <bin.cheng@arm.com>
PR tree-optimization/80153
* gcc.c-torture/execute/pr80153.c: New.
[-- Attachment #2: pr80153-20170328.txt --]
[-- Type: text/plain, Size: 6897 bytes --]
diff --git a/gcc/testsuite/gcc.c-torture/execute/pr80153.c b/gcc/testsuite/gcc.c-torture/execute/pr80153.c
new file mode 100644
index 0000000..3eed578
--- /dev/null
+++ b/gcc/testsuite/gcc.c-torture/execute/pr80153.c
@@ -0,0 +1,48 @@
+/* PR tree-optimization/80153 */
+
+void check (int, int, int) __attribute__((noinline));
+void check (int c, int c2, int val)
+{
+ if (!val) {
+ __builtin_abort();
+ }
+}
+
+static const char *buf;
+static int l, i;
+
+void _fputs(const char *str) __attribute__((noinline));
+void _fputs(const char *str)
+{
+ buf = str;
+ i = 0;
+ l = __builtin_strlen(buf);
+}
+
+char _fgetc() __attribute__((noinline));
+char _fgetc()
+{
+ char val = buf[i];
+ i++;
+ if (i > l)
+ return -1;
+ else
+ return val;
+}
+
+static const char *string = "oops!\n";
+
+int main(void)
+{
+ int i;
+ int c;
+
+ _fputs(string);
+
+ for (i = 0; i < __builtin_strlen(string); i++) {
+ c = _fgetc();
+ check(c, string[i], c == string[i]);
+ }
+
+ return 0;
+}
diff --git a/gcc/tree-affine.c b/gcc/tree-affine.c
index e620eea..a7f0b6a 100644
--- a/gcc/tree-affine.c
+++ b/gcc/tree-affine.c
@@ -370,27 +370,28 @@ tree_to_aff_combination (tree expr, tree type, aff_tree *comb)
aff_combination_elt (comb, type, expr);
}
-/* Creates EXPR + ELT * SCALE in TYPE. EXPR is taken from affine
+/* Creates EXPR + ELT * SCALE in COMB's type. EXPR is taken from affine
combination COMB. */
static tree
-add_elt_to_tree (tree expr, tree type, tree elt, const widest_int &scale_in,
- aff_tree *comb ATTRIBUTE_UNUSED)
+add_elt_to_tree (tree expr, tree elt, const widest_int &scale_in,
+ aff_tree *comb)
{
enum tree_code code;
- tree type1 = type;
- if (POINTER_TYPE_P (type))
- type1 = sizetype;
+ tree type = POINTER_TYPE_P (comb->type) ? sizetype : comb->type;
widest_int scale = wide_int_ext_for_comb (scale_in, comb);
if (scale == -1
+ && POINTER_TYPE_P (comb->type)
&& POINTER_TYPE_P (TREE_TYPE (elt)))
{
elt = convert_to_ptrofftype (elt);
elt = fold_build1 (NEGATE_EXPR, TREE_TYPE (elt), elt);
scale = 1;
}
+ else if (!POINTER_TYPE_P (comb->type))
+ elt = fold_convert (comb->type, elt);
if (scale == 1)
{
@@ -399,22 +400,20 @@ add_elt_to_tree (tree expr, tree type, tree elt, const widest_int &scale_in,
if (POINTER_TYPE_P (TREE_TYPE (elt)))
return elt;
else
- return fold_convert (type1, elt);
+ return fold_convert (type, elt);
}
if (POINTER_TYPE_P (TREE_TYPE (expr)))
return fold_build_pointer_plus (expr, elt);
if (POINTER_TYPE_P (TREE_TYPE (elt)))
return fold_build_pointer_plus (elt, expr);
- return fold_build2 (PLUS_EXPR, type1,
- expr, fold_convert (type1, elt));
+ return fold_build2 (PLUS_EXPR, type, expr, fold_convert (type, elt));
}
if (scale == -1)
{
if (!expr)
- return fold_build1 (NEGATE_EXPR, type1,
- fold_convert (type1, elt));
+ return fold_build1 (NEGATE_EXPR, type, fold_convert (type, elt));
if (POINTER_TYPE_P (TREE_TYPE (expr)))
{
@@ -422,14 +421,12 @@ add_elt_to_tree (tree expr, tree type, tree elt, const widest_int &scale_in,
elt = fold_build1 (NEGATE_EXPR, TREE_TYPE (elt), elt);
return fold_build_pointer_plus (expr, elt);
}
- return fold_build2 (MINUS_EXPR, type1,
- expr, fold_convert (type1, elt));
+ return fold_build2 (MINUS_EXPR, type, expr, fold_convert (type, elt));
}
- elt = fold_convert (type1, elt);
+ elt = fold_convert (type, elt);
if (!expr)
- return fold_build2 (MULT_EXPR, type1, elt,
- wide_int_to_tree (type1, scale));
+ return fold_build2 (MULT_EXPR, type, elt, wide_int_to_tree (type, scale));
if (wi::neg_p (scale))
{
@@ -439,15 +436,14 @@ add_elt_to_tree (tree expr, tree type, tree elt, const widest_int &scale_in,
else
code = PLUS_EXPR;
- elt = fold_build2 (MULT_EXPR, type1, elt,
- wide_int_to_tree (type1, scale));
+ elt = fold_build2 (MULT_EXPR, type, elt, wide_int_to_tree (type, scale));
if (POINTER_TYPE_P (TREE_TYPE (expr)))
{
if (code == MINUS_EXPR)
- elt = fold_build1 (NEGATE_EXPR, type1, elt);
+ elt = fold_build1 (NEGATE_EXPR, type, elt);
return fold_build_pointer_plus (expr, elt);
}
- return fold_build2 (code, type1, expr, elt);
+ return fold_build2 (code, type, expr, elt);
}
/* Makes tree from the affine combination COMB. */
@@ -455,22 +451,18 @@ add_elt_to_tree (tree expr, tree type, tree elt, const widest_int &scale_in,
tree
aff_combination_to_tree (aff_tree *comb)
{
- tree type = comb->type;
tree expr = NULL_TREE;
unsigned i;
widest_int off, sgn;
- tree type1 = type;
- if (POINTER_TYPE_P (type))
- type1 = sizetype;
+ tree type = POINTER_TYPE_P (comb->type) ? sizetype : comb->type;
gcc_assert (comb->n == MAX_AFF_ELTS || comb->rest == NULL_TREE);
for (i = 0; i < comb->n; i++)
- expr = add_elt_to_tree (expr, type, comb->elts[i].val, comb->elts[i].coef,
- comb);
+ expr = add_elt_to_tree (expr, comb->elts[i].val, comb->elts[i].coef, comb);
if (comb->rest)
- expr = add_elt_to_tree (expr, type, comb->rest, 1, comb);
+ expr = add_elt_to_tree (expr, comb->rest, 1, comb);
/* Ensure that we get x - 1, not x + (-1) or x + 0xff..f if x is
unsigned. */
@@ -484,8 +476,7 @@ aff_combination_to_tree (aff_tree *comb)
off = comb->offset;
sgn = 1;
}
- return add_elt_to_tree (expr, type, wide_int_to_tree (type1, off), sgn,
- comb);
+ return add_elt_to_tree (expr, wide_int_to_tree (type, off), sgn, comb);
}
/* Copies the tree elements of COMB to ensure that they are not shared. */
diff --git a/gcc/tree-ssa-loop-ivopts.c b/gcc/tree-ssa-loop-ivopts.c
index 8dc65881..fa993ab 100644
--- a/gcc/tree-ssa-loop-ivopts.c
+++ b/gcc/tree-ssa-loop-ivopts.c
@@ -1171,7 +1171,7 @@ alloc_iv (struct ivopts_data *data, tree base, tree step,
|| contain_complex_addr_expr (expr))
{
aff_tree comb;
- tree_to_aff_combination (expr, TREE_TYPE (base), &comb);
+ tree_to_aff_combination (expr, TREE_TYPE (expr), &comb);
base = fold_convert (TREE_TYPE (base), aff_combination_to_tree (&comb));
}
@@ -3787,6 +3787,12 @@ get_computation_aff (struct loop *loop,
overflows, as all the arithmetics will in the end be performed in UUTYPE
anyway. */
common_type = determine_common_wider_type (&ubase, &cbase);
+ /* We don't need to compute in UUTYPE if this is the original candidate,
+ and candidate/use have the same (pointer) type. */
+ if (ctype == utype && common_type == utype
+ && POINTER_TYPE_P (utype) && TYPE_UNSIGNED (utype)
+ && cand->pos == IP_ORIGINAL && cand->incremented_at == use->stmt)
+ uutype = utype;
/* use = ubase - ratio * cbase + ratio * var. */
tree_to_aff_combination (ubase, common_type, aff);
next prev parent reply other threads:[~2017-03-29 15:22 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-03-28 12:17 Bin Cheng
2017-03-28 12:39 ` Richard Biener
2017-03-29 15:32 ` Bin.Cheng [this message]
2017-03-30 10:46 ` Richard Biener
2017-03-30 12:44 ` Bin.Cheng
2017-03-30 13:00 ` Richard Biener
2017-03-30 13:20 ` Bin.Cheng
2017-03-30 13:34 ` Bin.Cheng
2017-03-30 14:37 ` Richard Biener
2017-04-05 7:25 ` Bin.Cheng
2017-04-05 7:26 ` Bin.Cheng
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=CAHFci2-8Rn_oQ0UpcAQr2ByAYX0KVZRVV_6irffK8BpXifSj1Q@mail.gmail.com \
--to=amker.cheng@gmail.com \
--cc=Bin.Cheng@arm.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=nd@arm.com \
--cc=richard.guenther@gmail.com \
/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).