* [RFC][PR64946] "abs" vectorization fails for char/short types
@ 2018-05-17 2:56 Kugan Vivekanandarajah
2018-05-17 4:35 ` Andrew Pinski
0 siblings, 1 reply; 11+ messages in thread
From: Kugan Vivekanandarajah @ 2018-05-17 2:56 UTC (permalink / raw)
To: GCC Patches
[-- Attachment #1: Type: text/plain, Size: 1164 bytes --]
As mentioned in the PR, I am trying to add ABSU_EXPR to fix this
issue. In the attached patch, in fold_cond_expr_with_comparison I am
generating ABSU_EXPR for these cases. As I understand, absu_expr is
well defined in RTL. So, the issue is generating absu_expr and
transferring to RTL in the correct way. I am not sure I am not doing
all that is needed. I will clean up and add more test-cases based on
the feedback.
Thanks,
Kugan
gcc/ChangeLog:
2018-05-13 Kugan Vivekanandarajah <kugan.vivekanandarajah@linaro.org>
* expr.c (expand_expr_real_2): Handle ABSU_EXPR.
* fold-const.c (fold_cond_expr_with_comparison): Generate ABSU_EXPR
(fold_unary_loc): Handle ABSU_EXPR.
* optabs-tree.c (optab_for_tree_code): Likewise.
* tree-cfg.c (verify_expr): Likewise.
(verify_gimple_assign_unary): Likewise.
* tree-if-conv.c (fold_build_cond_expr): Likewise.
* tree-inline.c (estimate_operator_cost): Likewise.
* tree-pretty-print.c (dump_generic_node): Likewise.
* tree.def (ABSU_EXPR): New.
gcc/testsuite/ChangeLog:
2018-05-13 Kugan Vivekanandarajah <kugan.vivekanandarajah@linaro.org>
* gcc.dg/absu.c: New test.
[-- Attachment #2: abs.txt --]
[-- Type: text/plain, Size: 5864 bytes --]
diff --git a/gcc/expr.c b/gcc/expr.c
index 5e3d9a5..67f8dd1 100644
--- a/gcc/expr.c
+++ b/gcc/expr.c
@@ -9063,6 +9063,7 @@ expand_expr_real_2 (sepops ops, rtx target, machine_mode tmode,
return REDUCE_BIT_FIELD (temp);
case ABS_EXPR:
+ case ABSU_EXPR:
op0 = expand_expr (treeop0, subtarget,
VOIDmode, EXPAND_NORMAL);
if (modifier == EXPAND_STACK_PARM)
@@ -9074,7 +9075,8 @@ expand_expr_real_2 (sepops ops, rtx target, machine_mode tmode,
/* Unsigned abs is simply the operand. Testing here means we don't
risk generating incorrect code below. */
- if (TYPE_UNSIGNED (type))
+ if (TYPE_UNSIGNED (type)
+ && (code != ABSU_EXPR))
return op0;
return expand_abs (mode, op0, target, unsignedp,
diff --git a/gcc/fold-const.c b/gcc/fold-const.c
index 3a99b66..6e80178 100644
--- a/gcc/fold-const.c
+++ b/gcc/fold-const.c
@@ -5324,8 +5324,17 @@ fold_cond_expr_with_comparison (location_t loc, tree type,
case GT_EXPR:
if (TYPE_UNSIGNED (TREE_TYPE (arg1)))
break;
- tem = fold_build1_loc (loc, ABS_EXPR, TREE_TYPE (arg1), arg1);
- return fold_convert_loc (loc, type, tem);
+ if (TREE_CODE (arg1) == NOP_EXPR)
+ {
+ arg1 = TREE_OPERAND (arg1, 0);
+ tem = fold_build1_loc (loc, ABSU_EXPR, unsigned_type_for (arg1_type), arg1);
+ return fold_convert_loc (loc, type, tem);
+ }
+ else
+ {
+ tem = fold_build1_loc (loc, ABS_EXPR, TREE_TYPE (arg1), arg1);
+ return fold_convert_loc (loc, type, tem);
+ }
case UNLE_EXPR:
case UNLT_EXPR:
if (flag_trapping_math)
@@ -7698,7 +7707,8 @@ fold_unary_loc (location_t loc, enum tree_code code, tree type, tree op0)
if (arg0)
{
if (CONVERT_EXPR_CODE_P (code)
- || code == FLOAT_EXPR || code == ABS_EXPR || code == NEGATE_EXPR)
+ || code == FLOAT_EXPR || code == ABS_EXPR
+ || code == ABSU_EXPR || code == NEGATE_EXPR)
{
/* Don't use STRIP_NOPS, because signedness of argument type
matters. */
diff --git a/gcc/optabs-tree.c b/gcc/optabs-tree.c
index 71e172c..2b812e5 100644
--- a/gcc/optabs-tree.c
+++ b/gcc/optabs-tree.c
@@ -235,6 +235,7 @@ optab_for_tree_code (enum tree_code code, const_tree type,
return trapv ? negv_optab : neg_optab;
case ABS_EXPR:
+ case ABSU_EXPR:
return trapv ? absv_optab : abs_optab;
default:
diff --git a/gcc/testsuite/gcc.dg/absu.c b/gcc/testsuite/gcc.dg/absu.c
index e69de29..43e651b 100644
--- a/gcc/testsuite/gcc.dg/absu.c
+++ b/gcc/testsuite/gcc.dg/absu.c
@@ -0,0 +1,39 @@
+
+/* { dg-do run } */
+/* { dg-options "-O0" } */
+
+#include <limits.h>
+#define ABS(x) (((x) >= 0) ? (x) : -(x))
+
+#define DEF_TEST(TYPE) \
+void foo_##TYPE (signed TYPE x, unsigned TYPE y){ \
+ TYPE t = ABS (x); \
+ if (t != y) \
+ __builtin_abort (); \
+} \
+
+DEF_TEST (char);
+DEF_TEST (short);
+DEF_TEST (int);
+DEF_TEST (long);
+void main ()
+{
+ foo_char (SCHAR_MIN + 1, SCHAR_MAX);
+ foo_char (0, 0);
+ foo_char (SCHAR_MAX, SCHAR_MAX);
+
+ foo_int (-1, 1);
+ foo_int (0, 0);
+ foo_int (INT_MAX, INT_MAX);
+ foo_int (INT_MIN + 1, INT_MAX);
+
+ foo_short (-1, 1);
+ foo_short (0, 0);
+ foo_short (SHRT_MAX, SHRT_MAX);
+ foo_short (SHRT_MIN + 1, SHRT_MAX);
+
+ foo_long (-1, 1);
+ foo_long (0, 0);
+ foo_long (LONG_MAX, LONG_MAX);
+ foo_long (LONG_MIN + 1, LONG_MAX);
+}
diff --git a/gcc/tree-cfg.c b/gcc/tree-cfg.c
index 9485f73..59a115c 100644
--- a/gcc/tree-cfg.c
+++ b/gcc/tree-cfg.c
@@ -3167,6 +3167,9 @@ verify_expr (tree *tp, int *walk_subtrees, void *data ATTRIBUTE_UNUSED)
CHECK_OP (0, "invalid operand to unary operator");
break;
+ case ABSU_EXPR:
+ break;
+
case REALPART_EXPR:
case IMAGPART_EXPR:
case BIT_FIELD_REF:
@@ -3937,6 +3940,9 @@ verify_gimple_assign_unary (gassign *stmt)
case PAREN_EXPR:
case CONJ_EXPR:
break;
+ case ABSU_EXPR:
+ /* FIXME. */
+ return false;
case VEC_DUPLICATE_EXPR:
if (TREE_CODE (lhs_type) != VECTOR_TYPE
diff --git a/gcc/tree-if-conv.c b/gcc/tree-if-conv.c
index 71dac4f..13d4c25 100644
--- a/gcc/tree-if-conv.c
+++ b/gcc/tree-if-conv.c
@@ -466,7 +466,8 @@ fold_build_cond_expr (tree type, tree cond, tree rhs, tree lhs)
if (is_gimple_val (cond_expr))
return cond_expr;
- if (TREE_CODE (cond_expr) == ABS_EXPR)
+ if (TREE_CODE (cond_expr) == ABS_EXPR
+ || TREE_CODE (cond_expr) == ABSU_EXPR)
{
rhs1 = TREE_OPERAND (cond_expr, 1);
STRIP_USELESS_TYPE_CONVERSION (rhs1);
diff --git a/gcc/tree-inline.c b/gcc/tree-inline.c
index 5a0a252..d272974 100644
--- a/gcc/tree-inline.c
+++ b/gcc/tree-inline.c
@@ -3866,6 +3866,7 @@ estimate_operator_cost (enum tree_code code, eni_weights *weights,
case MIN_EXPR:
case MAX_EXPR:
case ABS_EXPR:
+ case ABSU_EXPR:
case LSHIFT_EXPR:
case RSHIFT_EXPR:
diff --git a/gcc/tree-pretty-print.c b/gcc/tree-pretty-print.c
index 276ad00..b74d8c9 100644
--- a/gcc/tree-pretty-print.c
+++ b/gcc/tree-pretty-print.c
@@ -2460,6 +2460,12 @@ dump_generic_node (pretty_printer *pp, tree node, int spc, dump_flags_t flags,
pp_greater (pp);
break;
+ case ABSU_EXPR:
+ pp_string (pp, "ABSU_EXPR <");
+ dump_generic_node (pp, TREE_OPERAND (node, 0), spc, flags, false);
+ pp_greater (pp);
+ break;
+
case RANGE_EXPR:
NIY;
break;
diff --git a/gcc/tree.def b/gcc/tree.def
index 31de6c0..768167b 100644
--- a/gcc/tree.def
+++ b/gcc/tree.def
@@ -760,6 +760,7 @@ DEFTREECODE (MAX_EXPR, "max_expr", tcc_binary, 2)
An ABS_EXPR must have either an INTEGER_TYPE or a REAL_TYPE. The
operand of the ABS_EXPR must have the same type. */
DEFTREECODE (ABS_EXPR, "abs_expr", tcc_unary, 1)
+DEFTREECODE (ABSU_EXPR, "absu_expr", tcc_unary, 1)
/* Shift operations for shift and rotate.
Shift means logical shift if done on an
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFC][PR64946] "abs" vectorization fails for char/short types
2018-05-17 2:56 [RFC][PR64946] "abs" vectorization fails for char/short types Kugan Vivekanandarajah
@ 2018-05-17 4:35 ` Andrew Pinski
2018-05-17 10:38 ` Richard Biener
0 siblings, 1 reply; 11+ messages in thread
From: Andrew Pinski @ 2018-05-17 4:35 UTC (permalink / raw)
To: Kugan Vivekanandarajah; +Cc: GCC Patches
On Wed, May 16, 2018 at 7:14 PM, Kugan Vivekanandarajah
<kugan.vivekanandarajah@linaro.org> wrote:
> As mentioned in the PR, I am trying to add ABSU_EXPR to fix this
> issue. In the attached patch, in fold_cond_expr_with_comparison I am
> generating ABSU_EXPR for these cases. As I understand, absu_expr is
> well defined in RTL. So, the issue is generating absu_expr and
> transferring to RTL in the correct way. I am not sure I am not doing
> all that is needed. I will clean up and add more test-cases based on
> the feedback.
diff --git a/gcc/optabs-tree.c b/gcc/optabs-tree.c
index 71e172c..2b812e5 100644
--- a/gcc/optabs-tree.c
+++ b/gcc/optabs-tree.c
@@ -235,6 +235,7 @@ optab_for_tree_code (enum tree_code code, const_tree type,
return trapv ? negv_optab : neg_optab;
case ABS_EXPR:
+ case ABSU_EXPR:
return trapv ? absv_optab : abs_optab;
This part is not correct, it should something like this:
case ABS_EXPR:
return trapv ? absv_optab : abs_optab;
+ case ABSU_EXPR:
+ return abs_optab ;
Because ABSU is not undefined at the TYPE_MAX.
Thanks,
Andrew
>
> Thanks,
> Kugan
>
>
> gcc/ChangeLog:
>
> 2018-05-13 Kugan Vivekanandarajah <kugan.vivekanandarajah@linaro.org>
>
> * expr.c (expand_expr_real_2): Handle ABSU_EXPR.
> * fold-const.c (fold_cond_expr_with_comparison): Generate ABSU_EXPR
> (fold_unary_loc): Handle ABSU_EXPR.
> * optabs-tree.c (optab_for_tree_code): Likewise.
> * tree-cfg.c (verify_expr): Likewise.
> (verify_gimple_assign_unary): Likewise.
> * tree-if-conv.c (fold_build_cond_expr): Likewise.
> * tree-inline.c (estimate_operator_cost): Likewise.
> * tree-pretty-print.c (dump_generic_node): Likewise.
> * tree.def (ABSU_EXPR): New.
>
> gcc/testsuite/ChangeLog:
>
> 2018-05-13 Kugan Vivekanandarajah <kugan.vivekanandarajah@linaro.org>
>
> * gcc.dg/absu.c: New test.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFC][PR64946] "abs" vectorization fails for char/short types
2018-05-17 4:35 ` Andrew Pinski
@ 2018-05-17 10:38 ` Richard Biener
2018-05-18 7:28 ` Kugan Vivekanandarajah
0 siblings, 1 reply; 11+ messages in thread
From: Richard Biener @ 2018-05-17 10:38 UTC (permalink / raw)
To: Andrew Pinski; +Cc: kugan.vivekanandarajah, GCC Patches
On Thu, May 17, 2018 at 4:56 AM Andrew Pinski <pinskia@gmail.com> wrote:
> On Wed, May 16, 2018 at 7:14 PM, Kugan Vivekanandarajah
> <kugan.vivekanandarajah@linaro.org> wrote:
> > As mentioned in the PR, I am trying to add ABSU_EXPR to fix this
> > issue. In the attached patch, in fold_cond_expr_with_comparison I am
> > generating ABSU_EXPR for these cases. As I understand, absu_expr is
> > well defined in RTL. So, the issue is generating absu_expr and
> > transferring to RTL in the correct way. I am not sure I am not doing
> > all that is needed. I will clean up and add more test-cases based on
> > the feedback.
> diff --git a/gcc/optabs-tree.c b/gcc/optabs-tree.c
> index 71e172c..2b812e5 100644
> --- a/gcc/optabs-tree.c
> +++ b/gcc/optabs-tree.c
> @@ -235,6 +235,7 @@ optab_for_tree_code (enum tree_code code, const_tree
type,
> return trapv ? negv_optab : neg_optab;
> case ABS_EXPR:
> + case ABSU_EXPR:
> return trapv ? absv_optab : abs_optab;
> This part is not correct, it should something like this:
> case ABS_EXPR:
> return trapv ? absv_optab : abs_optab;
> + case ABSU_EXPR:
> + return abs_optab ;
> Because ABSU is not undefined at the TYPE_MAX.
Also
/* Unsigned abs is simply the operand. Testing here means we don't
risk generating incorrect code below. */
- if (TYPE_UNSIGNED (type))
+ if (TYPE_UNSIGNED (type)
+ && (code != ABSU_EXPR))
return op0;
is wrong. ABSU of an unsigned number is still just that number.
The change to fold_cond_expr_with_comparison looks odd to me
(premature optimization). It should be done separately - it seems
you are doing
(simplify (abs (convert @0)) (convert (absu @0)))
here.
You touch one other place in fold-const.c but there seem to be many
more that need ABSU_EXPR handling (you touched the one needed
for correctness) - esp. you should at least handle constant folding
in const_unop and the nonnegative predicate.
@@ -3167,6 +3167,9 @@ verify_expr (tree *tp, int *walk_subtrees, void *data
ATTRIBUTE_UNUSED)
CHECK_OP (0, "invalid operand to unary operator");
break;
+ case ABSU_EXPR:
+ break;
+
case REALPART_EXPR:
case IMAGPART_EXPR:
verify_expr is no more. Did you test this recently against trunk?
@@ -3937,6 +3940,9 @@ verify_gimple_assign_unary (gassign *stmt)
case PAREN_EXPR:
case CONJ_EXPR:
break;
+ case ABSU_EXPR:
+ /* FIXME. */
+ return false;
no - please not! Please add verification here - ABSU should be only
called on INTEGRAL, vector or complex INTEGRAL types and the
type of the LHS should be always the unsigned variant of the
argument type.
if (is_gimple_val (cond_expr))
return cond_expr;
- if (TREE_CODE (cond_expr) == ABS_EXPR)
+ if (TREE_CODE (cond_expr) == ABS_EXPR
+ || TREE_CODE (cond_expr) == ABSU_EXPR)
{
rhs1 = TREE_OPERAND (cond_expr, 1);
STRIP_USELESS_TYPE_CONVERSION (rhs1);
err, but the next line just builds a ABS_EXPR ...
How did you identify spots that need adjustment? I would expect that
once folding generates ABSU_EXPR that you need to adjust frontends
(C++ constexpr handling for example). Also I miss adjustments
to gimple-pretty-print.c and the GIMPLE FE parser.
recursively grepping throughout the whole gcc/ tree doesn't reveal too many
cases of ABS_EXPR so I think it's reasonable to audit all of them.
I also miss some trivial absu simplifications in match.pd. There are not
a lot of abs cases but similar ones would be good to have initially.
Thanks for tackling this!
Richard.
> Thanks,
> Andrew
> >
> > Thanks,
> > Kugan
> >
> >
> > gcc/ChangeLog:
> >
> > 2018-05-13 Kugan Vivekanandarajah <kugan.vivekanandarajah@linaro.org>
> >
> > * expr.c (expand_expr_real_2): Handle ABSU_EXPR.
> > * fold-const.c (fold_cond_expr_with_comparison): Generate ABSU_EXPR
> > (fold_unary_loc): Handle ABSU_EXPR.
> > * optabs-tree.c (optab_for_tree_code): Likewise.
> > * tree-cfg.c (verify_expr): Likewise.
> > (verify_gimple_assign_unary): Likewise.
> > * tree-if-conv.c (fold_build_cond_expr): Likewise.
> > * tree-inline.c (estimate_operator_cost): Likewise.
> > * tree-pretty-print.c (dump_generic_node): Likewise.
> > * tree.def (ABSU_EXPR): New.
> >
> > gcc/testsuite/ChangeLog:
> >
> > 2018-05-13 Kugan Vivekanandarajah <kugan.vivekanandarajah@linaro.org>
> >
> > * gcc.dg/absu.c: New test.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFC][PR64946] "abs" vectorization fails for char/short types
2018-05-17 10:38 ` Richard Biener
@ 2018-05-18 7:28 ` Kugan Vivekanandarajah
2018-05-18 8:01 ` Richard Biener
2018-06-01 2:12 ` Kugan Vivekanandarajah
0 siblings, 2 replies; 11+ messages in thread
From: Kugan Vivekanandarajah @ 2018-05-18 7:28 UTC (permalink / raw)
To: Richard Biener; +Cc: Andrew Pinski, GCC Patches
Hi Richard,
Thanks for the review. I am revising the patch based on Andrew's comments too.
On 17 May 2018 at 20:36, Richard Biener <richard.guenther@gmail.com> wrote:
> On Thu, May 17, 2018 at 4:56 AM Andrew Pinski <pinskia@gmail.com> wrote:
>
>> On Wed, May 16, 2018 at 7:14 PM, Kugan Vivekanandarajah
>> <kugan.vivekanandarajah@linaro.org> wrote:
>> > As mentioned in the PR, I am trying to add ABSU_EXPR to fix this
>> > issue. In the attached patch, in fold_cond_expr_with_comparison I am
>> > generating ABSU_EXPR for these cases. As I understand, absu_expr is
>> > well defined in RTL. So, the issue is generating absu_expr and
>> > transferring to RTL in the correct way. I am not sure I am not doing
>> > all that is needed. I will clean up and add more test-cases based on
>> > the feedback.
>
>
>> diff --git a/gcc/optabs-tree.c b/gcc/optabs-tree.c
>> index 71e172c..2b812e5 100644
>> --- a/gcc/optabs-tree.c
>> +++ b/gcc/optabs-tree.c
>> @@ -235,6 +235,7 @@ optab_for_tree_code (enum tree_code code, const_tree
> type,
>> return trapv ? negv_optab : neg_optab;
>
>> case ABS_EXPR:
>> + case ABSU_EXPR:
>> return trapv ? absv_optab : abs_optab;
>
>
>> This part is not correct, it should something like this:
>
>> case ABS_EXPR:
>> return trapv ? absv_optab : abs_optab;
>> + case ABSU_EXPR:
>> + return abs_optab ;
>
>> Because ABSU is not undefined at the TYPE_MAX.
>
> Also
>
> /* Unsigned abs is simply the operand. Testing here means we don't
> risk generating incorrect code below. */
> - if (TYPE_UNSIGNED (type))
> + if (TYPE_UNSIGNED (type)
> + && (code != ABSU_EXPR))
> return op0;
>
> is wrong. ABSU of an unsigned number is still just that number.
>
> The change to fold_cond_expr_with_comparison looks odd to me
> (premature optimization). It should be done separately - it seems
> you are doing
FE seems to be using this to generate ABS_EXPR from
c_fully_fold_internal to fold_build3_loc and so on. I changed this to
generate ABSU_EXPR for the case in the testcase. So the question
should be, in what cases do we need ABS_EXPR and in what cases do we
need ABSU_EXPR. It is not very clear to me.
>
> (simplify (abs (convert @0)) (convert (absu @0)))
>
> here.
>
> You touch one other place in fold-const.c but there seem to be many
> more that need ABSU_EXPR handling (you touched the one needed
> for correctness) - esp. you should at least handle constant folding
> in const_unop and the nonnegative predicate.
OK.
>
> @@ -3167,6 +3167,9 @@ verify_expr (tree *tp, int *walk_subtrees, void *data
> ATTRIBUTE_UNUSED)
> CHECK_OP (0, "invalid operand to unary operator");
> break;
>
> + case ABSU_EXPR:
> + break;
> +
> case REALPART_EXPR:
> case IMAGPART_EXPR:
>
> verify_expr is no more. Did you test this recently against trunk?
This patch is against slightly older trunk. I will rebase it.
>
> @@ -3937,6 +3940,9 @@ verify_gimple_assign_unary (gassign *stmt)
> case PAREN_EXPR:
> case CONJ_EXPR:
> break;
> + case ABSU_EXPR:
> + /* FIXME. */
> + return false;
>
> no - please not! Please add verification here - ABSU should be only
> called on INTEGRAL, vector or complex INTEGRAL types and the
> type of the LHS should be always the unsigned variant of the
> argument type.
OK.
>
> if (is_gimple_val (cond_expr))
> return cond_expr;
>
> - if (TREE_CODE (cond_expr) == ABS_EXPR)
> + if (TREE_CODE (cond_expr) == ABS_EXPR
> + || TREE_CODE (cond_expr) == ABSU_EXPR)
> {
> rhs1 = TREE_OPERAND (cond_expr, 1);
> STRIP_USELESS_TYPE_CONVERSION (rhs1);
>
> err, but the next line just builds a ABS_EXPR ...
>
> How did you identify spots that need adjustment? I would expect that
> once folding generates ABSU_EXPR that you need to adjust frontends
> (C++ constexpr handling for example). Also I miss adjustments
> to gimple-pretty-print.c and the GIMPLE FE parser.
I will add this.
>
> recursively grepping throughout the whole gcc/ tree doesn't reveal too many
> cases of ABS_EXPR so I think it's reasonable to audit all of them.
>
> I also miss some trivial absu simplifications in match.pd. There are not
> a lot of abs cases but similar ones would be good to have initially.
I will add them in the next version.
Thanks,
Kugan
>
> Thanks for tackling this!
> Richard.
>
>> Thanks,
>> Andrew
>
>> >
>> > Thanks,
>> > Kugan
>> >
>> >
>> > gcc/ChangeLog:
>> >
>> > 2018-05-13 Kugan Vivekanandarajah <kugan.vivekanandarajah@linaro.org>
>> >
>> > * expr.c (expand_expr_real_2): Handle ABSU_EXPR.
>> > * fold-const.c (fold_cond_expr_with_comparison): Generate ABSU_EXPR
>> > (fold_unary_loc): Handle ABSU_EXPR.
>> > * optabs-tree.c (optab_for_tree_code): Likewise.
>> > * tree-cfg.c (verify_expr): Likewise.
>> > (verify_gimple_assign_unary): Likewise.
>> > * tree-if-conv.c (fold_build_cond_expr): Likewise.
>> > * tree-inline.c (estimate_operator_cost): Likewise.
>> > * tree-pretty-print.c (dump_generic_node): Likewise.
>> > * tree.def (ABSU_EXPR): New.
>> >
>> > gcc/testsuite/ChangeLog:
>> >
>> > 2018-05-13 Kugan Vivekanandarajah <kugan.vivekanandarajah@linaro.org>
>> >
>> > * gcc.dg/absu.c: New test.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFC][PR64946] "abs" vectorization fails for char/short types
2018-05-18 7:28 ` Kugan Vivekanandarajah
@ 2018-05-18 8:01 ` Richard Biener
2018-06-01 2:12 ` Kugan Vivekanandarajah
1 sibling, 0 replies; 11+ messages in thread
From: Richard Biener @ 2018-05-18 8:01 UTC (permalink / raw)
To: kugan.vivekanandarajah; +Cc: Andrew Pinski, GCC Patches
On Fri, May 18, 2018 at 4:37 AM Kugan Vivekanandarajah <
kugan.vivekanandarajah@linaro.org> wrote:
> Hi Richard,
> Thanks for the review. I am revising the patch based on Andrew's comments
too.
> On 17 May 2018 at 20:36, Richard Biener <richard.guenther@gmail.com>
wrote:
> > On Thu, May 17, 2018 at 4:56 AM Andrew Pinski <pinskia@gmail.com> wrote:
> >
> >> On Wed, May 16, 2018 at 7:14 PM, Kugan Vivekanandarajah
> >> <kugan.vivekanandarajah@linaro.org> wrote:
> >> > As mentioned in the PR, I am trying to add ABSU_EXPR to fix this
> >> > issue. In the attached patch, in fold_cond_expr_with_comparison I am
> >> > generating ABSU_EXPR for these cases. As I understand, absu_expr is
> >> > well defined in RTL. So, the issue is generating absu_expr and
> >> > transferring to RTL in the correct way. I am not sure I am not doing
> >> > all that is needed. I will clean up and add more test-cases based on
> >> > the feedback.
> >
> >
> >> diff --git a/gcc/optabs-tree.c b/gcc/optabs-tree.c
> >> index 71e172c..2b812e5 100644
> >> --- a/gcc/optabs-tree.c
> >> +++ b/gcc/optabs-tree.c
> >> @@ -235,6 +235,7 @@ optab_for_tree_code (enum tree_code code,
const_tree
> > type,
> >> return trapv ? negv_optab : neg_optab;
> >
> >> case ABS_EXPR:
> >> + case ABSU_EXPR:
> >> return trapv ? absv_optab : abs_optab;
> >
> >
> >> This part is not correct, it should something like this:
> >
> >> case ABS_EXPR:
> >> return trapv ? absv_optab : abs_optab;
> >> + case ABSU_EXPR:
> >> + return abs_optab ;
> >
> >> Because ABSU is not undefined at the TYPE_MAX.
> >
> > Also
> >
> > /* Unsigned abs is simply the operand. Testing here means we
don't
> > risk generating incorrect code below. */
> > - if (TYPE_UNSIGNED (type))
> > + if (TYPE_UNSIGNED (type)
> > + && (code != ABSU_EXPR))
> > return op0;
> >
> > is wrong. ABSU of an unsigned number is still just that number.
> >
> > The change to fold_cond_expr_with_comparison looks odd to me
> > (premature optimization). It should be done separately - it seems
> > you are doing
> FE seems to be using this to generate ABS_EXPR from
> c_fully_fold_internal to fold_build3_loc and so on. I changed this to
> generate ABSU_EXPR for the case in the testcase. So the question
> should be, in what cases do we need ABS_EXPR and in what cases do we
> need ABSU_EXPR. It is not very clear to me.
Well, all existing ABS_EXPR generations are obviously fine. Then there
are transforms that are not possible w/o ABSU_EXPR like the narrowing
you performed here. This is becasue for ABS_EXPR you can't simply avoid
the undefined
behavior by performing the operation on unsigned integers... (that's similar
to signed integer division of -INT_MIN / -1 -- the result is representable
in
unsigned but not in signed).
> >
> > (simplify (abs (convert @0)) (convert (absu @0)))
> >
> > here.
> >
> > You touch one other place in fold-const.c but there seem to be many
> > more that need ABSU_EXPR handling (you touched the one needed
> > for correctness) - esp. you should at least handle constant folding
> > in const_unop and the nonnegative predicate.
> OK.
> >
> > @@ -3167,6 +3167,9 @@ verify_expr (tree *tp, int *walk_subtrees, void
*data
> > ATTRIBUTE_UNUSED)
> > CHECK_OP (0, "invalid operand to unary operator");
> > break;
> >
> > + case ABSU_EXPR:
> > + break;
> > +
> > case REALPART_EXPR:
> > case IMAGPART_EXPR:
> >
> > verify_expr is no more. Did you test this recently against trunk?
> This patch is against slightly older trunk. I will rebase it.
> >
> > @@ -3937,6 +3940,9 @@ verify_gimple_assign_unary (gassign *stmt)
> > case PAREN_EXPR:
> > case CONJ_EXPR:
> > break;
> > + case ABSU_EXPR:
> > + /* FIXME. */
> > + return false;
> >
> > no - please not! Please add verification here - ABSU should be only
> > called on INTEGRAL, vector or complex INTEGRAL types and the
> > type of the LHS should be always the unsigned variant of the
> > argument type.
> OK.
> >
> > if (is_gimple_val (cond_expr))
> > return cond_expr;
> >
> > - if (TREE_CODE (cond_expr) == ABS_EXPR)
> > + if (TREE_CODE (cond_expr) == ABS_EXPR
> > + || TREE_CODE (cond_expr) == ABSU_EXPR)
> > {
> > rhs1 = TREE_OPERAND (cond_expr, 1);
> > STRIP_USELESS_TYPE_CONVERSION (rhs1);
> >
> > err, but the next line just builds a ABS_EXPR ...
> >
> > How did you identify spots that need adjustment? I would expect that
> > once folding generates ABSU_EXPR that you need to adjust frontends
> > (C++ constexpr handling for example). Also I miss adjustments
> > to gimple-pretty-print.c and the GIMPLE FE parser.
> I will add this.
> >
> > recursively grepping throughout the whole gcc/ tree doesn't reveal too
many
> > cases of ABS_EXPR so I think it's reasonable to audit all of them.
> >
> > I also miss some trivial absu simplifications in match.pd. There are
not
> > a lot of abs cases but similar ones would be good to have initially.
> I will add them in the next version.
> Thanks,
> Kugan
> >
> > Thanks for tackling this!
> > Richard.
> >
> >> Thanks,
> >> Andrew
> >
> >> >
> >> > Thanks,
> >> > Kugan
> >> >
> >> >
> >> > gcc/ChangeLog:
> >> >
> >> > 2018-05-13 Kugan Vivekanandarajah <
kugan.vivekanandarajah@linaro.org>
> >> >
> >> > * expr.c (expand_expr_real_2): Handle ABSU_EXPR.
> >> > * fold-const.c (fold_cond_expr_with_comparison): Generate
ABSU_EXPR
> >> > (fold_unary_loc): Handle ABSU_EXPR.
> >> > * optabs-tree.c (optab_for_tree_code): Likewise.
> >> > * tree-cfg.c (verify_expr): Likewise.
> >> > (verify_gimple_assign_unary): Likewise.
> >> > * tree-if-conv.c (fold_build_cond_expr): Likewise.
> >> > * tree-inline.c (estimate_operator_cost): Likewise.
> >> > * tree-pretty-print.c (dump_generic_node): Likewise.
> >> > * tree.def (ABSU_EXPR): New.
> >> >
> >> > gcc/testsuite/ChangeLog:
> >> >
> >> > 2018-05-13 Kugan Vivekanandarajah <
kugan.vivekanandarajah@linaro.org>
> >> >
> >> > * gcc.dg/absu.c: New test.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFC][PR64946] "abs" vectorization fails for char/short types
2018-05-18 7:28 ` Kugan Vivekanandarajah
2018-05-18 8:01 ` Richard Biener
@ 2018-06-01 2:12 ` Kugan Vivekanandarajah
2018-06-01 12:21 ` Richard Biener
1 sibling, 1 reply; 11+ messages in thread
From: Kugan Vivekanandarajah @ 2018-06-01 2:12 UTC (permalink / raw)
To: Richard Biener; +Cc: Andrew Pinski, GCC Patches
[-- Attachment #1: Type: text/plain, Size: 6434 bytes --]
Hi Richard,
This is the revised patch based on the review and the discussion in
https://gcc.gnu.org/ml/gcc/2018-05/msg00179.html.
In summary:
- I skipped (element_precision (type) < element_precision (TREE_TYPE
(@0))) in the match.pd pattern as this would prevent transformation
for the case in PR.
that is, I am interested in is something like:
char t = (char) ABS_EXPR <(int) x>
and I want to generate
char t = (char) ABSU_EXPR <x>
- I also haven't added all the necessary match.pd changes for
ABSU_EXPR. I have a patch for that but will submit separately based on
this reveiw.
- I also tried to add ABSU_EXPRsupport in the places as necessary by
grepping for ABS_EXPR.
- I also had to add special casing in vectorizer for ABSU_EXP as its
result is unsigned type.
Is this OK. Patch bootstraps and the regression test is ongoing.
Thanks,
Kugan
On 18 May 2018 at 12:36, Kugan Vivekanandarajah
<kugan.vivekanandarajah@linaro.org> wrote:
> Hi Richard,
>
> Thanks for the review. I am revising the patch based on Andrew's comments too.
>
> On 17 May 2018 at 20:36, Richard Biener <richard.guenther@gmail.com> wrote:
>> On Thu, May 17, 2018 at 4:56 AM Andrew Pinski <pinskia@gmail.com> wrote:
>>
>>> On Wed, May 16, 2018 at 7:14 PM, Kugan Vivekanandarajah
>>> <kugan.vivekanandarajah@linaro.org> wrote:
>>> > As mentioned in the PR, I am trying to add ABSU_EXPR to fix this
>>> > issue. In the attached patch, in fold_cond_expr_with_comparison I am
>>> > generating ABSU_EXPR for these cases. As I understand, absu_expr is
>>> > well defined in RTL. So, the issue is generating absu_expr and
>>> > transferring to RTL in the correct way. I am not sure I am not doing
>>> > all that is needed. I will clean up and add more test-cases based on
>>> > the feedback.
>>
>>
>>> diff --git a/gcc/optabs-tree.c b/gcc/optabs-tree.c
>>> index 71e172c..2b812e5 100644
>>> --- a/gcc/optabs-tree.c
>>> +++ b/gcc/optabs-tree.c
>>> @@ -235,6 +235,7 @@ optab_for_tree_code (enum tree_code code, const_tree
>> type,
>>> return trapv ? negv_optab : neg_optab;
>>
>>> case ABS_EXPR:
>>> + case ABSU_EXPR:
>>> return trapv ? absv_optab : abs_optab;
>>
>>
>>> This part is not correct, it should something like this:
>>
>>> case ABS_EXPR:
>>> return trapv ? absv_optab : abs_optab;
>>> + case ABSU_EXPR:
>>> + return abs_optab ;
>>
>>> Because ABSU is not undefined at the TYPE_MAX.
>>
>> Also
>>
>> /* Unsigned abs is simply the operand. Testing here means we don't
>> risk generating incorrect code below. */
>> - if (TYPE_UNSIGNED (type))
>> + if (TYPE_UNSIGNED (type)
>> + && (code != ABSU_EXPR))
>> return op0;
>>
>> is wrong. ABSU of an unsigned number is still just that number.
>>
>> The change to fold_cond_expr_with_comparison looks odd to me
>> (premature optimization). It should be done separately - it seems
>> you are doing
>
> FE seems to be using this to generate ABS_EXPR from
> c_fully_fold_internal to fold_build3_loc and so on. I changed this to
> generate ABSU_EXPR for the case in the testcase. So the question
> should be, in what cases do we need ABS_EXPR and in what cases do we
> need ABSU_EXPR. It is not very clear to me.
>
>
>>
>> (simplify (abs (convert @0)) (convert (absu @0)))
>>
>> here.
>>
>> You touch one other place in fold-const.c but there seem to be many
>> more that need ABSU_EXPR handling (you touched the one needed
>> for correctness) - esp. you should at least handle constant folding
>> in const_unop and the nonnegative predicate.
>
> OK.
>>
>> @@ -3167,6 +3167,9 @@ verify_expr (tree *tp, int *walk_subtrees, void *data
>> ATTRIBUTE_UNUSED)
>> CHECK_OP (0, "invalid operand to unary operator");
>> break;
>>
>> + case ABSU_EXPR:
>> + break;
>> +
>> case REALPART_EXPR:
>> case IMAGPART_EXPR:
>>
>> verify_expr is no more. Did you test this recently against trunk?
>
> This patch is against slightly older trunk. I will rebase it.
>
>>
>> @@ -3937,6 +3940,9 @@ verify_gimple_assign_unary (gassign *stmt)
>> case PAREN_EXPR:
>> case CONJ_EXPR:
>> break;
>> + case ABSU_EXPR:
>> + /* FIXME. */
>> + return false;
>>
>> no - please not! Please add verification here - ABSU should be only
>> called on INTEGRAL, vector or complex INTEGRAL types and the
>> type of the LHS should be always the unsigned variant of the
>> argument type.
>
> OK.
>>
>> if (is_gimple_val (cond_expr))
>> return cond_expr;
>>
>> - if (TREE_CODE (cond_expr) == ABS_EXPR)
>> + if (TREE_CODE (cond_expr) == ABS_EXPR
>> + || TREE_CODE (cond_expr) == ABSU_EXPR)
>> {
>> rhs1 = TREE_OPERAND (cond_expr, 1);
>> STRIP_USELESS_TYPE_CONVERSION (rhs1);
>>
>> err, but the next line just builds a ABS_EXPR ...
>>
>> How did you identify spots that need adjustment? I would expect that
>> once folding generates ABSU_EXPR that you need to adjust frontends
>> (C++ constexpr handling for example). Also I miss adjustments
>> to gimple-pretty-print.c and the GIMPLE FE parser.
>
> I will add this.
>>
>> recursively grepping throughout the whole gcc/ tree doesn't reveal too many
>> cases of ABS_EXPR so I think it's reasonable to audit all of them.
>>
>> I also miss some trivial absu simplifications in match.pd. There are not
>> a lot of abs cases but similar ones would be good to have initially.
>
> I will add them in the next version.
>
> Thanks,
> Kugan
>
>>
>> Thanks for tackling this!
>> Richard.
>>
>>> Thanks,
>>> Andrew
>>
>>> >
>>> > Thanks,
>>> > Kugan
>>> >
>>> >
>>> > gcc/ChangeLog:
>>> >
>>> > 2018-05-13 Kugan Vivekanandarajah <kugan.vivekanandarajah@linaro.org>
>>> >
>>> > * expr.c (expand_expr_real_2): Handle ABSU_EXPR.
>>> > * fold-const.c (fold_cond_expr_with_comparison): Generate ABSU_EXPR
>>> > (fold_unary_loc): Handle ABSU_EXPR.
>>> > * optabs-tree.c (optab_for_tree_code): Likewise.
>>> > * tree-cfg.c (verify_expr): Likewise.
>>> > (verify_gimple_assign_unary): Likewise.
>>> > * tree-if-conv.c (fold_build_cond_expr): Likewise.
>>> > * tree-inline.c (estimate_operator_cost): Likewise.
>>> > * tree-pretty-print.c (dump_generic_node): Likewise.
>>> > * tree.def (ABSU_EXPR): New.
>>> >
>>> > gcc/testsuite/ChangeLog:
>>> >
>>> > 2018-05-13 Kugan Vivekanandarajah <kugan.vivekanandarajah@linaro.org>
>>> >
>>> > * gcc.dg/absu.c: New test.
[-- Attachment #2: 0001-absu-v2.patch --]
[-- Type: text/x-patch, Size: 14306 bytes --]
From 6675f0fe71b1de2c2def5c5d2bd150ae1e707b5c Mon Sep 17 00:00:00 2001
From: Kugan Vivekanandarajah <kugan.vivekanandarajah@linaro.org>
Date: Tue, 29 May 2018 10:26:23 +1000
Subject: [PATCH] absu v2
Change-Id: Id34948b4d585fbcdcb8ff6eb3728fd3cc6c41bfa
---
gcc/c-family/c-common.c | 1 +
gcc/c/c-typeck.c | 10 ++++++++++
gcc/c/gimple-parser.c | 9 ++++++++-
gcc/cfgexpand.c | 1 +
gcc/config/i386/i386.c | 1 +
gcc/cp/constexpr.c | 1 +
gcc/cp/cp-gimplify.c | 1 +
gcc/dojump.c | 1 +
gcc/expr.c | 3 ++-
gcc/fold-const.c | 6 +++++-
gcc/gimple-pretty-print.c | 7 +++++--
gcc/gimple-ssa-backprop.c | 2 ++
gcc/match.pd | 8 ++++++++
gcc/optabs-tree.c | 2 ++
gcc/testsuite/gcc.dg/absu.c | 41 ++++++++++++++++++++++++++++++++++++++
gcc/testsuite/gcc.dg/gimplefe-29.c | 11 ++++++++++
gcc/tree-cfg.c | 6 ++++++
gcc/tree-eh.c | 1 +
gcc/tree-inline.c | 1 +
gcc/tree-pretty-print.c | 6 ++++++
gcc/tree-vect-patterns.c | 3 ++-
gcc/tree-vect-stmts.c | 6 +++++-
gcc/tree.def | 1 +
23 files changed, 122 insertions(+), 7 deletions(-)
create mode 100644 gcc/testsuite/gcc.dg/absu.c
create mode 100644 gcc/testsuite/gcc.dg/gimplefe-29.c
diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
index 859eeb4..0e8efb5 100644
--- a/gcc/c-family/c-common.c
+++ b/gcc/c-family/c-common.c
@@ -3312,6 +3312,7 @@ c_common_truthvalue_conversion (location_t location, tree expr)
case NEGATE_EXPR:
case ABS_EXPR:
+ case ABSU_EXPR:
case FLOAT_EXPR:
case EXCESS_PRECISION_EXPR:
/* These don't change whether an object is nonzero or zero. */
diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c
index 45a4529..5bb6804 100644
--- a/gcc/c/c-typeck.c
+++ b/gcc/c/c-typeck.c
@@ -4314,6 +4314,16 @@ build_unary_op (location_t location, enum tree_code code, tree xarg,
arg = default_conversion (arg);
break;
+ case ABSU_EXPR:
+ if (!(typecode == INTEGER_TYPE))
+ {
+ error_at (location, "wrong type argument to absu");
+ return error_mark_node;
+ }
+ else if (!noconvert)
+ arg = default_conversion (arg);
+ break;
+
case CONJ_EXPR:
/* Conjugating a real value is a no-op, but allow it anyway. */
if (!(typecode == INTEGER_TYPE || typecode == REAL_TYPE
diff --git a/gcc/c/gimple-parser.c b/gcc/c/gimple-parser.c
index 8f1c442..1be5d14 100644
--- a/gcc/c/gimple-parser.c
+++ b/gcc/c/gimple-parser.c
@@ -328,7 +328,8 @@ c_parser_gimple_statement (c_parser *parser, gimple_seq *seq)
case CPP_NAME:
{
tree id = c_parser_peek_token (parser)->value;
- if (strcmp (IDENTIFIER_POINTER (id), "__ABS") == 0)
+ if (strcmp (IDENTIFIER_POINTER (id), "__ABS") == 0
+ || strcmp (IDENTIFIER_POINTER (id), "__ABSU") == 0)
goto build_unary_expr;
break;
}
@@ -638,6 +639,12 @@ c_parser_gimple_unary_expression (c_parser *parser)
op = c_parser_gimple_postfix_expression (parser);
return parser_build_unary_op (op_loc, ABS_EXPR, op);
}
+ else if (strcmp (IDENTIFIER_POINTER (id), "__ABSU") == 0)
+ {
+ c_parser_consume_token (parser);
+ op = c_parser_gimple_postfix_expression (parser);
+ return parser_build_unary_op (op_loc, ABSU_EXPR, op);
+ }
else
return c_parser_gimple_postfix_expression (parser);
}
diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c
index 5c323be..d06224d 100644
--- a/gcc/cfgexpand.c
+++ b/gcc/cfgexpand.c
@@ -4620,6 +4620,7 @@ expand_debug_expr (tree exp)
}
case ABS_EXPR:
+ case ABSU_EXPR:
return simplify_gen_unary (ABS, mode, op0, mode);
case NEGATE_EXPR:
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 637c105..f5a4856 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -51232,6 +51232,7 @@ ix86_add_stmt_cost (void *data, int count, enum vect_cost_for_stmt kind,
case BIT_IOR_EXPR:
case ABS_EXPR:
+ case ABSU_EXPR:
case MIN_EXPR:
case MAX_EXPR:
case BIT_XOR_EXPR:
diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
index a099408..30ea091 100644
--- a/gcc/cp/constexpr.c
+++ b/gcc/cp/constexpr.c
@@ -5759,6 +5759,7 @@ potential_constant_expression_1 (tree t, bool want_rval, bool strict, bool now,
case FLOAT_EXPR:
case NEGATE_EXPR:
case ABS_EXPR:
+ case ABSU_EXPR:
case TRUTH_NOT_EXPR:
case FIXED_CONVERT_EXPR:
case UNARY_PLUS_EXPR:
diff --git a/gcc/cp/cp-gimplify.c b/gcc/cp/cp-gimplify.c
index b4e23e2..4567365 100644
--- a/gcc/cp/cp-gimplify.c
+++ b/gcc/cp/cp-gimplify.c
@@ -2272,6 +2272,7 @@ cp_fold (tree x)
case FLOAT_EXPR:
case NEGATE_EXPR:
case ABS_EXPR:
+ case ABSU_EXPR:
case BIT_NOT_EXPR:
case TRUTH_NOT_EXPR:
case FIXED_CONVERT_EXPR:
diff --git a/gcc/dojump.c b/gcc/dojump.c
index 9da8a0e..88cc96a 100644
--- a/gcc/dojump.c
+++ b/gcc/dojump.c
@@ -467,6 +467,7 @@ do_jump (tree exp, rtx_code_label *if_false_label,
/* FALLTHRU */
case NON_LVALUE_EXPR:
case ABS_EXPR:
+ case ABSU_EXPR:
case NEGATE_EXPR:
case LROTATE_EXPR:
case RROTATE_EXPR:
diff --git a/gcc/expr.c b/gcc/expr.c
index ecc5292..a92c23f 100644
--- a/gcc/expr.c
+++ b/gcc/expr.c
@@ -9013,6 +9013,7 @@ expand_expr_real_2 (sepops ops, rtx target, machine_mode tmode,
return REDUCE_BIT_FIELD (temp);
case ABS_EXPR:
+ case ABSU_EXPR:
op0 = expand_expr (treeop0, subtarget,
VOIDmode, EXPAND_NORMAL);
if (modifier == EXPAND_STACK_PARM)
@@ -9024,7 +9025,7 @@ expand_expr_real_2 (sepops ops, rtx target, machine_mode tmode,
/* Unsigned abs is simply the operand. Testing here means we don't
risk generating incorrect code below. */
- if (TYPE_UNSIGNED (type))
+ if (TYPE_UNSIGNED (TREE_TYPE (treeop0)))
return op0;
return expand_abs (mode, op0, target, unsignedp,
diff --git a/gcc/fold-const.c b/gcc/fold-const.c
index 0f57f07..20b4271 100644
--- a/gcc/fold-const.c
+++ b/gcc/fold-const.c
@@ -1723,7 +1723,8 @@ const_unop (enum tree_code code, tree type, tree arg0)
&& HONOR_SNANS (TYPE_MODE (TREE_TYPE (arg0)))
&& REAL_VALUE_ISSIGNALING_NAN (TREE_REAL_CST (arg0))
&& code != NEGATE_EXPR
- && code != ABS_EXPR)
+ && code != ABS_EXPR
+ && code != ABSU_EXPR)
return NULL_TREE;
switch (code)
@@ -1761,6 +1762,9 @@ const_unop (enum tree_code code, tree type, tree arg0)
if (TREE_CODE (arg0) == INTEGER_CST || TREE_CODE (arg0) == REAL_CST)
return fold_abs_const (arg0, type);
break;
+ case ABSU_EXPR:
+ return fold_convert (type, fold_abs_const (arg0,
+ signed_type_for (type)));
case CONJ_EXPR:
if (TREE_CODE (arg0) == COMPLEX_CST)
diff --git a/gcc/gimple-pretty-print.c b/gcc/gimple-pretty-print.c
index 49e9e12..bb4d259 100644
--- a/gcc/gimple-pretty-print.c
+++ b/gcc/gimple-pretty-print.c
@@ -358,14 +358,17 @@ dump_unary_rhs (pretty_printer *buffer, gassign *gs, int spc,
break;
case ABS_EXPR:
+ case ABSU_EXPR:
if (flags & TDF_GIMPLE)
{
- pp_string (buffer, "__ABS ");
+ pp_string (buffer,
+ rhs_code == ABS_EXPR ? "__ABS " : "__ABSU ");
dump_generic_node (buffer, rhs, spc, flags, false);
}
else
{
- pp_string (buffer, "ABS_EXPR <");
+ pp_string (buffer,
+ rhs_code == ABS_EXPR ? "ABS_EXPR <" : "ABSU_EXPR <");
dump_generic_node (buffer, rhs, spc, flags, false);
pp_greater (buffer);
}
diff --git a/gcc/gimple-ssa-backprop.c b/gcc/gimple-ssa-backprop.c
index 9ab655c..bb378a9 100644
--- a/gcc/gimple-ssa-backprop.c
+++ b/gcc/gimple-ssa-backprop.c
@@ -408,6 +408,7 @@ backprop::process_assign_use (gassign *assign, tree rhs, usage_info *info)
switch (gimple_assign_rhs_code (assign))
{
case ABS_EXPR:
+ case ABSU_EXPR:
/* The sign of the input doesn't matter. */
info->flags.ignore_sign = true;
break;
@@ -675,6 +676,7 @@ strip_sign_op_1 (tree rhs)
switch (gimple_assign_rhs_code (assign))
{
case ABS_EXPR:
+ case ABSU_EXPR:
case NEGATE_EXPR:
return gimple_assign_rhs1 (assign);
diff --git a/gcc/match.pd b/gcc/match.pd
index 14386da..7d7c132 100644
--- a/gcc/match.pd
+++ b/gcc/match.pd
@@ -102,6 +102,14 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
(match (nop_convert @0)
@0)
+(simplify (abs (convert @0))
+ (if (ANY_INTEGRAL_TYPE_P (TREE_TYPE (@0))
+ && !TYPE_UNSIGNED (TREE_TYPE (@0))
+ && !TYPE_UNSIGNED (type))
+ (with { tree utype = unsigned_type_for (TREE_TYPE (@0)); }
+ (convert (absu:utype @0)))))
+
+
/* Simplifications of operations with one constant operand and
simplifications to constants or single values. */
diff --git a/gcc/optabs-tree.c b/gcc/optabs-tree.c
index 73e6654..092663b 100644
--- a/gcc/optabs-tree.c
+++ b/gcc/optabs-tree.c
@@ -234,6 +234,8 @@ optab_for_tree_code (enum tree_code code, const_tree type,
case ABS_EXPR:
return trapv ? absv_optab : abs_optab;
+ case ABSU_EXPR:
+ return abs_optab;
default:
return unknown_optab;
}
diff --git a/gcc/testsuite/gcc.dg/absu.c b/gcc/testsuite/gcc.dg/absu.c
new file mode 100644
index 0000000..81e37a4
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/absu.c
@@ -0,0 +1,41 @@
+
+/* { dg-do run } */
+/* { dg-options "-O0" } */
+
+#include <limits.h>
+#define ABS(x) (((x) >= 0) ? (x) : -(x))
+
+#define DEF_TEST(TYPE) \
+void foo_##TYPE (signed TYPE x, unsigned TYPE y){ \
+ TYPE t = ABS (x); \
+ if (t != y) \
+ __builtin_abort (); \
+} \
+
+DEF_TEST (char);
+DEF_TEST (short);
+DEF_TEST (int);
+DEF_TEST (long);
+void main ()
+{
+ foo_char (SCHAR_MIN + 1, SCHAR_MAX);
+ foo_char (0, 0);
+ foo_char (-1, 1);
+ foo_char (1, 1);
+ foo_char (SCHAR_MAX, SCHAR_MAX);
+
+ foo_int (-1, 1);
+ foo_int (0, 0);
+ foo_int (INT_MAX, INT_MAX);
+ foo_int (INT_MIN + 1, INT_MAX);
+
+ foo_short (-1, 1);
+ foo_short (0, 0);
+ foo_short (SHRT_MAX, SHRT_MAX);
+ foo_short (SHRT_MIN + 1, SHRT_MAX);
+
+ foo_long (-1, 1);
+ foo_long (0, 0);
+ foo_long (LONG_MAX, LONG_MAX);
+ foo_long (LONG_MIN + 1, LONG_MAX);
+}
diff --git a/gcc/testsuite/gcc.dg/gimplefe-29.c b/gcc/testsuite/gcc.dg/gimplefe-29.c
new file mode 100644
index 0000000..54b86ef
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/gimplefe-29.c
@@ -0,0 +1,11 @@
+/* { dg-do compile } */
+/* { dg-options "-O -fgimple -fdump-tree-ssa-gimple" } */
+
+unsigned int __GIMPLE() f(int a)
+{
+ unsigned int t0;
+ t0_1 = __ABSU a;
+ return t0_1;
+}
+
+/* { dg-final { scan-tree-dump "__ABSU a" "ssa" } } */
diff --git a/gcc/tree-cfg.c b/gcc/tree-cfg.c
index 68f4fd3..9b62583 100644
--- a/gcc/tree-cfg.c
+++ b/gcc/tree-cfg.c
@@ -3685,6 +3685,12 @@ verify_gimple_assign_unary (gassign *stmt)
case PAREN_EXPR:
case CONJ_EXPR:
break;
+ case ABSU_EXPR:
+ if (!TYPE_UNSIGNED (lhs_type)
+ || !ANY_INTEGRAL_TYPE_P (rhs1_type))
+ return true;
+ return false;
+ break;
case VEC_DUPLICATE_EXPR:
if (TREE_CODE (lhs_type) != VECTOR_TYPE
diff --git a/gcc/tree-eh.c b/gcc/tree-eh.c
index 30c6d9e..44b1399 100644
--- a/gcc/tree-eh.c
+++ b/gcc/tree-eh.c
@@ -2465,6 +2465,7 @@ operation_could_trap_helper_p (enum tree_code op,
case NEGATE_EXPR:
case ABS_EXPR:
+ case ABSU_EXPR:
case CONJ_EXPR:
/* These operations don't trap with floating point. */
if (honor_trapv)
diff --git a/gcc/tree-inline.c b/gcc/tree-inline.c
index 7881131..665e53c 100644
--- a/gcc/tree-inline.c
+++ b/gcc/tree-inline.c
@@ -3865,6 +3865,7 @@ estimate_operator_cost (enum tree_code code, eni_weights *weights,
case MIN_EXPR:
case MAX_EXPR:
case ABS_EXPR:
+ case ABSU_EXPR:
case LSHIFT_EXPR:
case RSHIFT_EXPR:
diff --git a/gcc/tree-pretty-print.c b/gcc/tree-pretty-print.c
index 5a8c8eb..a950a9e 100644
--- a/gcc/tree-pretty-print.c
+++ b/gcc/tree-pretty-print.c
@@ -2463,6 +2463,12 @@ dump_generic_node (pretty_printer *pp, tree node, int spc, dump_flags_t flags,
pp_greater (pp);
break;
+ case ABSU_EXPR:
+ pp_string (pp, "ABSU_EXPR <");
+ dump_generic_node (pp, TREE_OPERAND (node, 0), spc, flags, false);
+ pp_greater (pp);
+ break;
+
case RANGE_EXPR:
NIY;
break;
diff --git a/gcc/tree-vect-patterns.c b/gcc/tree-vect-patterns.c
index 6da784c..af6b9bd 100644
--- a/gcc/tree-vect-patterns.c
+++ b/gcc/tree-vect-patterns.c
@@ -615,7 +615,8 @@ vect_recog_sad_pattern (vec<gimple *> *stmts, tree *type_in,
gcc_assert (abs_stmt_vinfo);
if (STMT_VINFO_DEF_TYPE (abs_stmt_vinfo) != vect_internal_def)
return NULL;
- if (gimple_assign_rhs_code (abs_stmt) != ABS_EXPR)
+ if (gimple_assign_rhs_code (abs_stmt) != ABS_EXPR
+ && gimple_assign_rhs_code (abs_stmt) != ABSU_EXPR)
return NULL;
tree abs_oprnd = gimple_assign_rhs1 (abs_stmt);
diff --git a/gcc/tree-vect-stmts.c b/gcc/tree-vect-stmts.c
index 66c78de..b52d714 100644
--- a/gcc/tree-vect-stmts.c
+++ b/gcc/tree-vect-stmts.c
@@ -5995,7 +5995,11 @@ vectorizable_operation (gimple *stmt, gimple_stmt_iterator *gsi,
"transform binary/unary operation.\n");
/* Handle def. */
- vec_dest = vect_create_destination_var (scalar_dest, vectype);
+ if (code == ABSU_EXPR)
+ vec_dest = vect_create_destination_var (scalar_dest,
+ unsigned_type_for (vectype));
+ else
+ vec_dest = vect_create_destination_var (scalar_dest, vectype);
/* POINTER_DIFF_EXPR has pointer arguments which are vectorized as
vectors with unsigned elements, but the result is signed. So, we
diff --git a/gcc/tree.def b/gcc/tree.def
index c660b2c..5fec781 100644
--- a/gcc/tree.def
+++ b/gcc/tree.def
@@ -760,6 +760,7 @@ DEFTREECODE (MAX_EXPR, "max_expr", tcc_binary, 2)
An ABS_EXPR must have either an INTEGER_TYPE or a REAL_TYPE. The
operand of the ABS_EXPR must have the same type. */
DEFTREECODE (ABS_EXPR, "abs_expr", tcc_unary, 1)
+DEFTREECODE (ABSU_EXPR, "absu_expr", tcc_unary, 1)
/* Shift operations for shift and rotate.
Shift means logical shift if done on an
--
2.7.4
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFC][PR64946] "abs" vectorization fails for char/short types
2018-06-01 2:12 ` Kugan Vivekanandarajah
@ 2018-06-01 12:21 ` Richard Biener
2018-06-04 8:18 ` Kugan Vivekanandarajah
0 siblings, 1 reply; 11+ messages in thread
From: Richard Biener @ 2018-06-01 12:21 UTC (permalink / raw)
To: kugan.vivekanandarajah; +Cc: Andrew Pinski, GCC Patches
On Fri, Jun 1, 2018 at 4:12 AM Kugan Vivekanandarajah
<kugan.vivekanandarajah@linaro.org> wrote:
>
> Hi Richard,
>
> This is the revised patch based on the review and the discussion in
> https://gcc.gnu.org/ml/gcc/2018-05/msg00179.html.
>
> In summary:
> - I skipped (element_precision (type) < element_precision (TREE_TYPE
> (@0))) in the match.pd pattern as this would prevent transformation
> for the case in PR.
> that is, I am interested in is something like:
> char t = (char) ABS_EXPR <(int) x>
> and I want to generate
> char t = (char) ABSU_EXPR <x>
>
> - I also haven't added all the necessary match.pd changes for
> ABSU_EXPR. I have a patch for that but will submit separately based on
> this reveiw.
>
> - I also tried to add ABSU_EXPRsupport in the places as necessary by
> grepping for ABS_EXPR.
>
> - I also had to add special casing in vectorizer for ABSU_EXP as its
> result is unsigned type.
>
> Is this OK. Patch bootstraps and the regression test is ongoing.
The c/c-typeck.c:build_unary_op change looks unnecessary - the
C FE should never generate this directly (the c-common one might
be triggered by early folding I guess).
@@ -1761,6 +1762,9 @@ const_unop (enum tree_code code, tree type, tree arg0)
if (TREE_CODE (arg0) == INTEGER_CST || TREE_CODE (arg0) == REAL_CST)
return fold_abs_const (arg0, type);
break;
+ case ABSU_EXPR:
+ return fold_convert (type, fold_abs_const (arg0,
+ signed_type_for (type)));
case CONJ_EXPR:
I think this will get you bogus TREE_OVERFLOW flags set on ABSU (-INT_MIN).
I think you want to change fold_abs_const to properly deal with arg0 being
signed and type unsigned. That is, sth like
diff --git a/gcc/fold-const.c b/gcc/fold-const.c
index 6f80f1b1d69..f60f9c77e91 100644
--- a/gcc/fold-const.c
+++ b/gcc/fold-const.c
@@ -13843,18 +13843,19 @@ fold_abs_const (tree arg0, tree type)
{
/* If the value is unsigned or non-negative, then the absolute value
is the same as the ordinary value. */
- if (!wi::neg_p (wi::to_wide (arg0), TYPE_SIGN (type)))
- t = arg0;
+ wide_int val = wi::to_wide (arg0);
+ bool overflow = false;
+ if (!wi::neg_p (val, TYPE_SIGN (TREE_TYPE (arg0))))
+ ;
/* If the value is negative, then the absolute value is
its negation. */
else
- {
- bool overflow;
- wide_int val = wi::neg (wi::to_wide (arg0), &overflow);
- t = force_fit_type (type, val, -1,
- overflow | TREE_OVERFLOW (arg0));
- }
+ wide_int val = wi::neg (val, &overflow);
+
+ /* Force to the destination type, set TREE_OVERFLOW for signed
+ TYPE only. */
+ t = force_fit_type (type, val, 1, overflow | TREE_OVERFLOW (arg0));
}
break;
and then simply share the const_unop code with ABS_EXPR.
diff --git a/gcc/match.pd b/gcc/match.pd
index 14386da..7d7c132 100644
--- a/gcc/match.pd
+++ b/gcc/match.pd
@@ -102,6 +102,14 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
(match (nop_convert @0)
@0)
+(simplify (abs (convert @0))
+ (if (ANY_INTEGRAL_TYPE_P (TREE_TYPE (@0))
+ && !TYPE_UNSIGNED (TREE_TYPE (@0))
+ && !TYPE_UNSIGNED (type))
+ (with { tree utype = unsigned_type_for (TREE_TYPE (@0)); }
+ (convert (absu:utype @0)))))
+
+
please put a comment before the pattern. I believe there's no
need to check for !TYPE_UNSIGNED (type). Note this
also converts abs ((char)int-var) to (char)absu(int-var) which
doesn't make sense. The original issue you want to address
here is the case where TYPE_PRECISION of @0 is less than
the precision of type. That is, you want to remove language
introduced integer promotion of @0 which only is possible
with ABSU. So please do add such precision check
(I simply suggested the bogus direction of the test).
diff --git a/gcc/tree-cfg.c b/gcc/tree-cfg.c
index 68f4fd3..9b62583 100644
--- a/gcc/tree-cfg.c
+++ b/gcc/tree-cfg.c
@@ -3685,6 +3685,12 @@ verify_gimple_assign_unary (gassign *stmt)
case PAREN_EXPR:
case CONJ_EXPR:
break;
+ case ABSU_EXPR:
+ if (!TYPE_UNSIGNED (lhs_type)
+ || !ANY_INTEGRAL_TYPE_P (rhs1_type))
if (!ANY_INTEGRAL_TYPE_P (lhs_type)
|| !TYPE_UNSIGNED (lhs_type)
|| !ANY_INTEGRAL_TYPE_P (rhs1_type)
|| TYPE_UNSIGNED (rhs1_type)
|| element_precision (lhs_type) != element_precision (rhs1_type))
{
error ("invalid types for ABSU_EXPR");
debug_generic_expr (lhs_type);
debug_generic_expr (rhs1_type);
return true;
}
+ return true;
+ return false;
+ break;
diff --git a/gcc/tree-eh.c b/gcc/tree-eh.c
index 30c6d9e..44b1399 100644
--- a/gcc/tree-eh.c
+++ b/gcc/tree-eh.c
@@ -2465,6 +2465,7 @@ operation_could_trap_helper_p (enum tree_code op,
case NEGATE_EXPR:
case ABS_EXPR:
+ case ABSU_EXPR:
case CONJ_EXPR:
/* These operations don't trap with floating point. */
if (honor_trapv)
ABSU never traps. Please instead unconditionally return false.
diff --git a/gcc/tree-vect-stmts.c b/gcc/tree-vect-stmts.c
index 66c78de..b52d714 100644
--- a/gcc/tree-vect-stmts.c
+++ b/gcc/tree-vect-stmts.c
@@ -5995,7 +5995,11 @@ vectorizable_operation (gimple *stmt,
gimple_stmt_iterator *gsi,
"transform binary/unary operation.\n");
/* Handle def. */
- vec_dest = vect_create_destination_var (scalar_dest, vectype);
+ if (code == ABSU_EXPR)
+ vec_dest = vect_create_destination_var (scalar_dest,
+ unsigned_type_for (vectype));
+ else
+ vec_dest = vect_create_destination_var (scalar_dest, vectype);
/* POINTER_DIFF_EXPR has pointer arguments which are vectorized as
vectors with unsigned elements, but the result is signed. So, we
simply use vectype_out for creation of vec_dest.
diff --git a/gcc/tree.def b/gcc/tree.def
index c660b2c..5fec781 100644
--- a/gcc/tree.def
+++ b/gcc/tree.def
@@ -760,6 +760,7 @@ DEFTREECODE (MAX_EXPR, "max_expr", tcc_binary, 2)
An ABS_EXPR must have either an INTEGER_TYPE or a REAL_TYPE. The
operand of the ABS_EXPR must have the same type. */
DEFTREECODE (ABS_EXPR, "abs_expr", tcc_unary, 1)
+DEFTREECODE (ABSU_EXPR, "absu_expr", tcc_unary, 1)
/* Shift operations for shift and rotate.
Shift means logical shift if done on an
You can clearly see that the comment before ABS_EXPR doesn't apply to ABSU_EXPR
so please add an appropriate one. I suggest
/* Represents the unsigned absolute value of the operand.
An ABSU_EXPR must have unsigned INTEGER_TYPE. The operand of the ABSU_EXPR
must have the corresponding signed type. */
Otherwise looks OK. (I didn't explicitely check for missing ABSU_EXPR
handling this time)
Thanks,
Richard.
> Thanks,
> Kugan
>
>
> On 18 May 2018 at 12:36, Kugan Vivekanandarajah
> <kugan.vivekanandarajah@linaro.org> wrote:
> > Hi Richard,
> >
> > Thanks for the review. I am revising the patch based on Andrew's comments too.
> >
> > On 17 May 2018 at 20:36, Richard Biener <richard.guenther@gmail.com> wrote:
> >> On Thu, May 17, 2018 at 4:56 AM Andrew Pinski <pinskia@gmail.com> wrote:
> >>
> >>> On Wed, May 16, 2018 at 7:14 PM, Kugan Vivekanandarajah
> >>> <kugan.vivekanandarajah@linaro.org> wrote:
> >>> > As mentioned in the PR, I am trying to add ABSU_EXPR to fix this
> >>> > issue. In the attached patch, in fold_cond_expr_with_comparison I am
> >>> > generating ABSU_EXPR for these cases. As I understand, absu_expr is
> >>> > well defined in RTL. So, the issue is generating absu_expr and
> >>> > transferring to RTL in the correct way. I am not sure I am not doing
> >>> > all that is needed. I will clean up and add more test-cases based on
> >>> > the feedback.
> >>
> >>
> >>> diff --git a/gcc/optabs-tree.c b/gcc/optabs-tree.c
> >>> index 71e172c..2b812e5 100644
> >>> --- a/gcc/optabs-tree.c
> >>> +++ b/gcc/optabs-tree.c
> >>> @@ -235,6 +235,7 @@ optab_for_tree_code (enum tree_code code, const_tree
> >> type,
> >>> return trapv ? negv_optab : neg_optab;
> >>
> >>> case ABS_EXPR:
> >>> + case ABSU_EXPR:
> >>> return trapv ? absv_optab : abs_optab;
> >>
> >>
> >>> This part is not correct, it should something like this:
> >>
> >>> case ABS_EXPR:
> >>> return trapv ? absv_optab : abs_optab;
> >>> + case ABSU_EXPR:
> >>> + return abs_optab ;
> >>
> >>> Because ABSU is not undefined at the TYPE_MAX.
> >>
> >> Also
> >>
> >> /* Unsigned abs is simply the operand. Testing here means we don't
> >> risk generating incorrect code below. */
> >> - if (TYPE_UNSIGNED (type))
> >> + if (TYPE_UNSIGNED (type)
> >> + && (code != ABSU_EXPR))
> >> return op0;
> >>
> >> is wrong. ABSU of an unsigned number is still just that number.
> >>
> >> The change to fold_cond_expr_with_comparison looks odd to me
> >> (premature optimization). It should be done separately - it seems
> >> you are doing
> >
> > FE seems to be using this to generate ABS_EXPR from
> > c_fully_fold_internal to fold_build3_loc and so on. I changed this to
> > generate ABSU_EXPR for the case in the testcase. So the question
> > should be, in what cases do we need ABS_EXPR and in what cases do we
> > need ABSU_EXPR. It is not very clear to me.
> >
> >
> >>
> >> (simplify (abs (convert @0)) (convert (absu @0)))
> >>
> >> here.
> >>
> >> You touch one other place in fold-const.c but there seem to be many
> >> more that need ABSU_EXPR handling (you touched the one needed
> >> for correctness) - esp. you should at least handle constant folding
> >> in const_unop and the nonnegative predicate.
> >
> > OK.
> >>
> >> @@ -3167,6 +3167,9 @@ verify_expr (tree *tp, int *walk_subtrees, void *data
> >> ATTRIBUTE_UNUSED)
> >> CHECK_OP (0, "invalid operand to unary operator");
> >> break;
> >>
> >> + case ABSU_EXPR:
> >> + break;
> >> +
> >> case REALPART_EXPR:
> >> case IMAGPART_EXPR:
> >>
> >> verify_expr is no more. Did you test this recently against trunk?
> >
> > This patch is against slightly older trunk. I will rebase it.
> >
> >>
> >> @@ -3937,6 +3940,9 @@ verify_gimple_assign_unary (gassign *stmt)
> >> case PAREN_EXPR:
> >> case CONJ_EXPR:
> >> break;
> >> + case ABSU_EXPR:
> >> + /* FIXME. */
> >> + return false;
> >>
> >> no - please not! Please add verification here - ABSU should be only
> >> called on INTEGRAL, vector or complex INTEGRAL types and the
> >> type of the LHS should be always the unsigned variant of the
> >> argument type.
> >
> > OK.
> >>
> >> if (is_gimple_val (cond_expr))
> >> return cond_expr;
> >>
> >> - if (TREE_CODE (cond_expr) == ABS_EXPR)
> >> + if (TREE_CODE (cond_expr) == ABS_EXPR
> >> + || TREE_CODE (cond_expr) == ABSU_EXPR)
> >> {
> >> rhs1 = TREE_OPERAND (cond_expr, 1);
> >> STRIP_USELESS_TYPE_CONVERSION (rhs1);
> >>
> >> err, but the next line just builds a ABS_EXPR ...
> >>
> >> How did you identify spots that need adjustment? I would expect that
> >> once folding generates ABSU_EXPR that you need to adjust frontends
> >> (C++ constexpr handling for example). Also I miss adjustments
> >> to gimple-pretty-print.c and the GIMPLE FE parser.
> >
> > I will add this.
> >>
> >> recursively grepping throughout the whole gcc/ tree doesn't reveal too many
> >> cases of ABS_EXPR so I think it's reasonable to audit all of them.
> >>
> >> I also miss some trivial absu simplifications in match.pd. There are not
> >> a lot of abs cases but similar ones would be good to have initially.
> >
> > I will add them in the next version.
> >
> > Thanks,
> > Kugan
> >
> >>
> >> Thanks for tackling this!
> >> Richard.
> >>
> >>> Thanks,
> >>> Andrew
> >>
> >>> >
> >>> > Thanks,
> >>> > Kugan
> >>> >
> >>> >
> >>> > gcc/ChangeLog:
> >>> >
> >>> > 2018-05-13 Kugan Vivekanandarajah <kugan.vivekanandarajah@linaro.org>
> >>> >
> >>> > * expr.c (expand_expr_real_2): Handle ABSU_EXPR.
> >>> > * fold-const.c (fold_cond_expr_with_comparison): Generate ABSU_EXPR
> >>> > (fold_unary_loc): Handle ABSU_EXPR.
> >>> > * optabs-tree.c (optab_for_tree_code): Likewise.
> >>> > * tree-cfg.c (verify_expr): Likewise.
> >>> > (verify_gimple_assign_unary): Likewise.
> >>> > * tree-if-conv.c (fold_build_cond_expr): Likewise.
> >>> > * tree-inline.c (estimate_operator_cost): Likewise.
> >>> > * tree-pretty-print.c (dump_generic_node): Likewise.
> >>> > * tree.def (ABSU_EXPR): New.
> >>> >
> >>> > gcc/testsuite/ChangeLog:
> >>> >
> >>> > 2018-05-13 Kugan Vivekanandarajah <kugan.vivekanandarajah@linaro.org>
> >>> >
> >>> > * gcc.dg/absu.c: New test.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFC][PR64946] "abs" vectorization fails for char/short types
2018-06-01 12:21 ` Richard Biener
@ 2018-06-04 8:18 ` Kugan Vivekanandarajah
2018-06-04 8:39 ` Richard Biener
0 siblings, 1 reply; 11+ messages in thread
From: Kugan Vivekanandarajah @ 2018-06-04 8:18 UTC (permalink / raw)
To: Richard Biener; +Cc: Andrew Pinski, GCC Patches
[-- Attachment #1: Type: text/plain, Size: 13548 bytes --]
Hi Richard,
Thanks for the review.
On 1 June 2018 at 22:20, Richard Biener <richard.guenther@gmail.com> wrote:
> On Fri, Jun 1, 2018 at 4:12 AM Kugan Vivekanandarajah
> <kugan.vivekanandarajah@linaro.org> wrote:
>>
>> Hi Richard,
>>
>> This is the revised patch based on the review and the discussion in
>> https://gcc.gnu.org/ml/gcc/2018-05/msg00179.html.
>>
>> In summary:
>> - I skipped (element_precision (type) < element_precision (TREE_TYPE
>> (@0))) in the match.pd pattern as this would prevent transformation
>> for the case in PR.
>> that is, I am interested in is something like:
>> char t = (char) ABS_EXPR <(int) x>
>> and I want to generate
>> char t = (char) ABSU_EXPR <x>
>>
>> - I also haven't added all the necessary match.pd changes for
>> ABSU_EXPR. I have a patch for that but will submit separately based on
>> this reveiw.
>>
>> - I also tried to add ABSU_EXPRsupport in the places as necessary by
>> grepping for ABS_EXPR.
>>
>> - I also had to add special casing in vectorizer for ABSU_EXP as its
>> result is unsigned type.
>>
>> Is this OK. Patch bootstraps and the regression test is ongoing.
>
> The c/c-typeck.c:build_unary_op change looks unnecessary - the
> C FE should never generate this directly (the c-common one might
> be triggered by early folding I guess).
The Gimple FE testcase is running into this.
>
> @@ -1761,6 +1762,9 @@ const_unop (enum tree_code code, tree type, tree arg0)
> if (TREE_CODE (arg0) == INTEGER_CST || TREE_CODE (arg0) == REAL_CST)
> return fold_abs_const (arg0, type);
> break;
> + case ABSU_EXPR:
> + return fold_convert (type, fold_abs_const (arg0,
> + signed_type_for (type)));
>
> case CONJ_EXPR:
>
> I think this will get you bogus TREE_OVERFLOW flags set on ABSU (-INT_MIN).
>
> I think you want to change fold_abs_const to properly deal with arg0 being
> signed and type unsigned. That is, sth like
>
> diff --git a/gcc/fold-const.c b/gcc/fold-const.c
> index 6f80f1b1d69..f60f9c77e91 100644
> --- a/gcc/fold-const.c
> +++ b/gcc/fold-const.c
> @@ -13843,18 +13843,19 @@ fold_abs_const (tree arg0, tree type)
> {
> /* If the value is unsigned or non-negative, then the absolute value
> is the same as the ordinary value. */
> - if (!wi::neg_p (wi::to_wide (arg0), TYPE_SIGN (type)))
> - t = arg0;
> + wide_int val = wi::to_wide (arg0);
> + bool overflow = false;
> + if (!wi::neg_p (val, TYPE_SIGN (TREE_TYPE (arg0))))
> + ;
>
> /* If the value is negative, then the absolute value is
> its negation. */
> else
> - {
> - bool overflow;
> - wide_int val = wi::neg (wi::to_wide (arg0), &overflow);
> - t = force_fit_type (type, val, -1,
> - overflow | TREE_OVERFLOW (arg0));
> - }
> + wide_int val = wi::neg (val, &overflow);
> +
> + /* Force to the destination type, set TREE_OVERFLOW for signed
> + TYPE only. */
> + t = force_fit_type (type, val, 1, overflow | TREE_OVERFLOW (arg0));
> }
> break;
>
> and then simply share the const_unop code with ABS_EXPR.
Done.
> diff --git a/gcc/match.pd b/gcc/match.pd
> index 14386da..7d7c132 100644
> --- a/gcc/match.pd
> +++ b/gcc/match.pd
> @@ -102,6 +102,14 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
> (match (nop_convert @0)
> @0)
>
> +(simplify (abs (convert @0))
> + (if (ANY_INTEGRAL_TYPE_P (TREE_TYPE (@0))
> + && !TYPE_UNSIGNED (TREE_TYPE (@0))
> + && !TYPE_UNSIGNED (type))
> + (with { tree utype = unsigned_type_for (TREE_TYPE (@0)); }
> + (convert (absu:utype @0)))))
> +
> +
>
> please put a comment before the pattern. I believe there's no
> need to check for !TYPE_UNSIGNED (type). Note this
> also converts abs ((char)int-var) to (char)absu(int-var) which
> doesn't make sense. The original issue you want to address
> here is the case where TYPE_PRECISION of @0 is less than
> the precision of type. That is, you want to remove language
> introduced integer promotion of @0 which only is possible
> with ABSU. So please do add such precision check
> (I simply suggested the bogus direction of the test).
Done.
>
> diff --git a/gcc/tree-cfg.c b/gcc/tree-cfg.c
> index 68f4fd3..9b62583 100644
> --- a/gcc/tree-cfg.c
> +++ b/gcc/tree-cfg.c
> @@ -3685,6 +3685,12 @@ verify_gimple_assign_unary (gassign *stmt)
> case PAREN_EXPR:
> case CONJ_EXPR:
> break;
> + case ABSU_EXPR:
> + if (!TYPE_UNSIGNED (lhs_type)
> + || !ANY_INTEGRAL_TYPE_P (rhs1_type))
>
> if (!ANY_INTEGRAL_TYPE_P (lhs_type)
> || !TYPE_UNSIGNED (lhs_type)
> || !ANY_INTEGRAL_TYPE_P (rhs1_type)
> || TYPE_UNSIGNED (rhs1_type)
> || element_precision (lhs_type) != element_precision (rhs1_type))
> {
> error ("invalid types for ABSU_EXPR");
> debug_generic_expr (lhs_type);
> debug_generic_expr (rhs1_type);
> return true;
> }
>
> + return true;
> + return false;
> + break;
>
> diff --git a/gcc/tree-eh.c b/gcc/tree-eh.c
> index 30c6d9e..44b1399 100644
> --- a/gcc/tree-eh.c
> +++ b/gcc/tree-eh.c
> @@ -2465,6 +2465,7 @@ operation_could_trap_helper_p (enum tree_code op,
>
> case NEGATE_EXPR:
> case ABS_EXPR:
> + case ABSU_EXPR:
> case CONJ_EXPR:
> /* These operations don't trap with floating point. */
> if (honor_trapv)
>
> ABSU never traps. Please instead unconditionally return false.
Done.
>
> diff --git a/gcc/tree-vect-stmts.c b/gcc/tree-vect-stmts.c
> index 66c78de..b52d714 100644
> --- a/gcc/tree-vect-stmts.c
> +++ b/gcc/tree-vect-stmts.c
> @@ -5995,7 +5995,11 @@ vectorizable_operation (gimple *stmt,
> gimple_stmt_iterator *gsi,
> "transform binary/unary operation.\n");
>
> /* Handle def. */
> - vec_dest = vect_create_destination_var (scalar_dest, vectype);
> + if (code == ABSU_EXPR)
> + vec_dest = vect_create_destination_var (scalar_dest,
> + unsigned_type_for (vectype));
> + else
> + vec_dest = vect_create_destination_var (scalar_dest, vectype);
>
> /* POINTER_DIFF_EXPR has pointer arguments which are vectorized as
> vectors with unsigned elements, but the result is signed. So, we
>
> simply use vectype_out for creation of vec_dest.
Done.
>
> diff --git a/gcc/tree.def b/gcc/tree.def
> index c660b2c..5fec781 100644
> --- a/gcc/tree.def
> +++ b/gcc/tree.def
> @@ -760,6 +760,7 @@ DEFTREECODE (MAX_EXPR, "max_expr", tcc_binary, 2)
> An ABS_EXPR must have either an INTEGER_TYPE or a REAL_TYPE. The
> operand of the ABS_EXPR must have the same type. */
> DEFTREECODE (ABS_EXPR, "abs_expr", tcc_unary, 1)
> +DEFTREECODE (ABSU_EXPR, "absu_expr", tcc_unary, 1)
>
> /* Shift operations for shift and rotate.
> Shift means logical shift if done on an
>
> You can clearly see that the comment before ABS_EXPR doesn't apply to ABSU_EXPR
> so please add an appropriate one. I suggest
>
> /* Represents the unsigned absolute value of the operand.
> An ABSU_EXPR must have unsigned INTEGER_TYPE. The operand of the ABSU_EXPR
> must have the corresponding signed type. */
Done.
Here is the reviesed patch. Is this OK?
Thanks,
Kugan
>
> Otherwise looks OK. (I didn't explicitely check for missing ABSU_EXPR
> handling this time)
>
> Thanks,
> Richard.
>
>
>> Thanks,
>> Kugan
>>
>>
>> On 18 May 2018 at 12:36, Kugan Vivekanandarajah
>> <kugan.vivekanandarajah@linaro.org> wrote:
>> > Hi Richard,
>> >
>> > Thanks for the review. I am revising the patch based on Andrew's comments too.
>> >
>> > On 17 May 2018 at 20:36, Richard Biener <richard.guenther@gmail.com> wrote:
>> >> On Thu, May 17, 2018 at 4:56 AM Andrew Pinski <pinskia@gmail.com> wrote:
>> >>
>> >>> On Wed, May 16, 2018 at 7:14 PM, Kugan Vivekanandarajah
>> >>> <kugan.vivekanandarajah@linaro.org> wrote:
>> >>> > As mentioned in the PR, I am trying to add ABSU_EXPR to fix this
>> >>> > issue. In the attached patch, in fold_cond_expr_with_comparison I am
>> >>> > generating ABSU_EXPR for these cases. As I understand, absu_expr is
>> >>> > well defined in RTL. So, the issue is generating absu_expr and
>> >>> > transferring to RTL in the correct way. I am not sure I am not doing
>> >>> > all that is needed. I will clean up and add more test-cases based on
>> >>> > the feedback.
>> >>
>> >>
>> >>> diff --git a/gcc/optabs-tree.c b/gcc/optabs-tree.c
>> >>> index 71e172c..2b812e5 100644
>> >>> --- a/gcc/optabs-tree.c
>> >>> +++ b/gcc/optabs-tree.c
>> >>> @@ -235,6 +235,7 @@ optab_for_tree_code (enum tree_code code, const_tree
>> >> type,
>> >>> return trapv ? negv_optab : neg_optab;
>> >>
>> >>> case ABS_EXPR:
>> >>> + case ABSU_EXPR:
>> >>> return trapv ? absv_optab : abs_optab;
>> >>
>> >>
>> >>> This part is not correct, it should something like this:
>> >>
>> >>> case ABS_EXPR:
>> >>> return trapv ? absv_optab : abs_optab;
>> >>> + case ABSU_EXPR:
>> >>> + return abs_optab ;
>> >>
>> >>> Because ABSU is not undefined at the TYPE_MAX.
>> >>
>> >> Also
>> >>
>> >> /* Unsigned abs is simply the operand. Testing here means we don't
>> >> risk generating incorrect code below. */
>> >> - if (TYPE_UNSIGNED (type))
>> >> + if (TYPE_UNSIGNED (type)
>> >> + && (code != ABSU_EXPR))
>> >> return op0;
>> >>
>> >> is wrong. ABSU of an unsigned number is still just that number.
>> >>
>> >> The change to fold_cond_expr_with_comparison looks odd to me
>> >> (premature optimization). It should be done separately - it seems
>> >> you are doing
>> >
>> > FE seems to be using this to generate ABS_EXPR from
>> > c_fully_fold_internal to fold_build3_loc and so on. I changed this to
>> > generate ABSU_EXPR for the case in the testcase. So the question
>> > should be, in what cases do we need ABS_EXPR and in what cases do we
>> > need ABSU_EXPR. It is not very clear to me.
>> >
>> >
>> >>
>> >> (simplify (abs (convert @0)) (convert (absu @0)))
>> >>
>> >> here.
>> >>
>> >> You touch one other place in fold-const.c but there seem to be many
>> >> more that need ABSU_EXPR handling (you touched the one needed
>> >> for correctness) - esp. you should at least handle constant folding
>> >> in const_unop and the nonnegative predicate.
>> >
>> > OK.
>> >>
>> >> @@ -3167,6 +3167,9 @@ verify_expr (tree *tp, int *walk_subtrees, void *data
>> >> ATTRIBUTE_UNUSED)
>> >> CHECK_OP (0, "invalid operand to unary operator");
>> >> break;
>> >>
>> >> + case ABSU_EXPR:
>> >> + break;
>> >> +
>> >> case REALPART_EXPR:
>> >> case IMAGPART_EXPR:
>> >>
>> >> verify_expr is no more. Did you test this recently against trunk?
>> >
>> > This patch is against slightly older trunk. I will rebase it.
>> >
>> >>
>> >> @@ -3937,6 +3940,9 @@ verify_gimple_assign_unary (gassign *stmt)
>> >> case PAREN_EXPR:
>> >> case CONJ_EXPR:
>> >> break;
>> >> + case ABSU_EXPR:
>> >> + /* FIXME. */
>> >> + return false;
>> >>
>> >> no - please not! Please add verification here - ABSU should be only
>> >> called on INTEGRAL, vector or complex INTEGRAL types and the
>> >> type of the LHS should be always the unsigned variant of the
>> >> argument type.
>> >
>> > OK.
>> >>
>> >> if (is_gimple_val (cond_expr))
>> >> return cond_expr;
>> >>
>> >> - if (TREE_CODE (cond_expr) == ABS_EXPR)
>> >> + if (TREE_CODE (cond_expr) == ABS_EXPR
>> >> + || TREE_CODE (cond_expr) == ABSU_EXPR)
>> >> {
>> >> rhs1 = TREE_OPERAND (cond_expr, 1);
>> >> STRIP_USELESS_TYPE_CONVERSION (rhs1);
>> >>
>> >> err, but the next line just builds a ABS_EXPR ...
>> >>
>> >> How did you identify spots that need adjustment? I would expect that
>> >> once folding generates ABSU_EXPR that you need to adjust frontends
>> >> (C++ constexpr handling for example). Also I miss adjustments
>> >> to gimple-pretty-print.c and the GIMPLE FE parser.
>> >
>> > I will add this.
>> >>
>> >> recursively grepping throughout the whole gcc/ tree doesn't reveal too many
>> >> cases of ABS_EXPR so I think it's reasonable to audit all of them.
>> >>
>> >> I also miss some trivial absu simplifications in match.pd. There are not
>> >> a lot of abs cases but similar ones would be good to have initially.
>> >
>> > I will add them in the next version.
>> >
>> > Thanks,
>> > Kugan
>> >
>> >>
>> >> Thanks for tackling this!
>> >> Richard.
>> >>
>> >>> Thanks,
>> >>> Andrew
>> >>
>> >>> >
>> >>> > Thanks,
>> >>> > Kugan
>> >>> >
>> >>> >
>> >>> > gcc/ChangeLog:
>> >>> >
>> >>> > 2018-05-13 Kugan Vivekanandarajah <kugan.vivekanandarajah@linaro.org>
>> >>> >
>> >>> > * expr.c (expand_expr_real_2): Handle ABSU_EXPR.
>> >>> > * fold-const.c (fold_cond_expr_with_comparison): Generate ABSU_EXPR
>> >>> > (fold_unary_loc): Handle ABSU_EXPR.
>> >>> > * optabs-tree.c (optab_for_tree_code): Likewise.
>> >>> > * tree-cfg.c (verify_expr): Likewise.
>> >>> > (verify_gimple_assign_unary): Likewise.
>> >>> > * tree-if-conv.c (fold_build_cond_expr): Likewise.
>> >>> > * tree-inline.c (estimate_operator_cost): Likewise.
>> >>> > * tree-pretty-print.c (dump_generic_node): Likewise.
>> >>> > * tree.def (ABSU_EXPR): New.
>> >>> >
>> >>> > gcc/testsuite/ChangeLog:
>> >>> >
>> >>> > 2018-05-13 Kugan Vivekanandarajah <kugan.vivekanandarajah@linaro.org>
>> >>> >
>> >>> > * gcc.dg/absu.c: New test.
[-- Attachment #2: absu_v3.txt --]
[-- Type: text/plain, Size: 15462 bytes --]
diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
index 859eeb4..0e8efb5 100644
--- a/gcc/c-family/c-common.c
+++ b/gcc/c-family/c-common.c
@@ -3312,6 +3312,7 @@ c_common_truthvalue_conversion (location_t location, tree expr)
case NEGATE_EXPR:
case ABS_EXPR:
+ case ABSU_EXPR:
case FLOAT_EXPR:
case EXCESS_PRECISION_EXPR:
/* These don't change whether an object is nonzero or zero. */
diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c
index 45a4529..5bb6804 100644
--- a/gcc/c/c-typeck.c
+++ b/gcc/c/c-typeck.c
@@ -4314,6 +4314,16 @@ build_unary_op (location_t location, enum tree_code code, tree xarg,
arg = default_conversion (arg);
break;
+ case ABSU_EXPR:
+ if (!(typecode == INTEGER_TYPE))
+ {
+ error_at (location, "wrong type argument to absu");
+ return error_mark_node;
+ }
+ else if (!noconvert)
+ arg = default_conversion (arg);
+ break;
+
case CONJ_EXPR:
/* Conjugating a real value is a no-op, but allow it anyway. */
if (!(typecode == INTEGER_TYPE || typecode == REAL_TYPE
diff --git a/gcc/c/gimple-parser.c b/gcc/c/gimple-parser.c
index c9abe24..11e76ff 100644
--- a/gcc/c/gimple-parser.c
+++ b/gcc/c/gimple-parser.c
@@ -328,7 +328,8 @@ c_parser_gimple_statement (c_parser *parser, gimple_seq *seq)
case CPP_NAME:
{
tree id = c_parser_peek_token (parser)->value;
- if (strcmp (IDENTIFIER_POINTER (id), "__ABS") == 0)
+ if (strcmp (IDENTIFIER_POINTER (id), "__ABS") == 0
+ || strcmp (IDENTIFIER_POINTER (id), "__ABSU") == 0)
goto build_unary_expr;
break;
}
@@ -638,6 +639,12 @@ c_parser_gimple_unary_expression (c_parser *parser)
op = c_parser_gimple_postfix_expression (parser);
return parser_build_unary_op (op_loc, ABS_EXPR, op);
}
+ else if (strcmp (IDENTIFIER_POINTER (id), "__ABSU") == 0)
+ {
+ c_parser_consume_token (parser);
+ op = c_parser_gimple_postfix_expression (parser);
+ return parser_build_unary_op (op_loc, ABSU_EXPR, op);
+ }
else
return c_parser_gimple_postfix_expression (parser);
}
diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c
index ef143a3..ba4543c 100644
--- a/gcc/cfgexpand.c
+++ b/gcc/cfgexpand.c
@@ -4621,6 +4621,7 @@ expand_debug_expr (tree exp)
}
case ABS_EXPR:
+ case ABSU_EXPR:
return simplify_gen_unary (ABS, mode, op0, mode);
case NEGATE_EXPR:
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index e19864d..487127c 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -51228,6 +51228,7 @@ ix86_add_stmt_cost (void *data, int count, enum vect_cost_for_stmt kind,
case BIT_IOR_EXPR:
case ABS_EXPR:
+ case ABSU_EXPR:
case MIN_EXPR:
case MAX_EXPR:
case BIT_XOR_EXPR:
diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
index 8c6ec55..bea9147 100644
--- a/gcc/cp/constexpr.c
+++ b/gcc/cp/constexpr.c
@@ -5760,6 +5760,7 @@ potential_constant_expression_1 (tree t, bool want_rval, bool strict, bool now,
case FLOAT_EXPR:
case NEGATE_EXPR:
case ABS_EXPR:
+ case ABSU_EXPR:
case TRUTH_NOT_EXPR:
case FIXED_CONVERT_EXPR:
case UNARY_PLUS_EXPR:
diff --git a/gcc/cp/cp-gimplify.c b/gcc/cp/cp-gimplify.c
index eda5f05..bca0c59 100644
--- a/gcc/cp/cp-gimplify.c
+++ b/gcc/cp/cp-gimplify.c
@@ -2271,6 +2271,7 @@ cp_fold (tree x)
case FLOAT_EXPR:
case NEGATE_EXPR:
case ABS_EXPR:
+ case ABSU_EXPR:
case BIT_NOT_EXPR:
case TRUTH_NOT_EXPR:
case FIXED_CONVERT_EXPR:
diff --git a/gcc/dojump.c b/gcc/dojump.c
index 9da8a0e..88cc96a 100644
--- a/gcc/dojump.c
+++ b/gcc/dojump.c
@@ -467,6 +467,7 @@ do_jump (tree exp, rtx_code_label *if_false_label,
/* FALLTHRU */
case NON_LVALUE_EXPR:
case ABS_EXPR:
+ case ABSU_EXPR:
case NEGATE_EXPR:
case LROTATE_EXPR:
case RROTATE_EXPR:
diff --git a/gcc/expr.c b/gcc/expr.c
index 00a802c..9efa535 100644
--- a/gcc/expr.c
+++ b/gcc/expr.c
@@ -9074,6 +9074,7 @@ expand_expr_real_2 (sepops ops, rtx target, machine_mode tmode,
return REDUCE_BIT_FIELD (temp);
case ABS_EXPR:
+ case ABSU_EXPR:
op0 = expand_expr (treeop0, subtarget,
VOIDmode, EXPAND_NORMAL);
if (modifier == EXPAND_STACK_PARM)
@@ -9085,7 +9086,7 @@ expand_expr_real_2 (sepops ops, rtx target, machine_mode tmode,
/* Unsigned abs is simply the operand. Testing here means we don't
risk generating incorrect code below. */
- if (TYPE_UNSIGNED (type))
+ if (TYPE_UNSIGNED (TREE_TYPE (treeop0)))
return op0;
return expand_abs (mode, op0, target, unsignedp,
diff --git a/gcc/fold-const.c b/gcc/fold-const.c
index faa184a..c19614e 100644
--- a/gcc/fold-const.c
+++ b/gcc/fold-const.c
@@ -1723,7 +1723,8 @@ const_unop (enum tree_code code, tree type, tree arg0)
&& HONOR_SNANS (TYPE_MODE (TREE_TYPE (arg0)))
&& REAL_VALUE_ISSIGNALING_NAN (TREE_REAL_CST (arg0))
&& code != NEGATE_EXPR
- && code != ABS_EXPR)
+ && code != ABS_EXPR
+ && code != ABSU_EXPR)
return NULL_TREE;
switch (code)
@@ -1758,6 +1759,7 @@ const_unop (enum tree_code code, tree type, tree arg0)
}
case ABS_EXPR:
+ case ABSU_EXPR:
if (TREE_CODE (arg0) == INTEGER_CST || TREE_CODE (arg0) == REAL_CST)
return fold_abs_const (arg0, type);
break;
@@ -13846,20 +13848,21 @@ fold_abs_const (tree arg0, tree type)
{
/* If the value is unsigned or non-negative, then the absolute value
is the same as the ordinary value. */
- if (!wi::neg_p (wi::to_wide (arg0), TYPE_SIGN (type)))
- t = arg0;
+ wide_int val = wi::to_wide (arg0);
+ bool overflow = false;
+ if (!wi::neg_p (val, TYPE_SIGN (TREE_TYPE (arg0))))
+ ;
/* If the value is negative, then the absolute value is
its negation. */
else
- {
- bool overflow;
- wide_int val = wi::neg (wi::to_wide (arg0), &overflow);
- t = force_fit_type (type, val, -1,
- overflow | TREE_OVERFLOW (arg0));
- }
+ val = wi::neg (val, &overflow);
+
+ /* Force to the destination type, set TREE_OVERFLOW for signed
+ TYPE only. */
+ t = force_fit_type (type, val, 1, overflow | TREE_OVERFLOW (arg0));
}
- break;
+ break;
case REAL_CST:
if (REAL_VALUE_NEGATIVE (TREE_REAL_CST (arg0)))
diff --git a/gcc/gimple-pretty-print.c b/gcc/gimple-pretty-print.c
index afe0147..4fa992d 100644
--- a/gcc/gimple-pretty-print.c
+++ b/gcc/gimple-pretty-print.c
@@ -358,14 +358,17 @@ dump_unary_rhs (pretty_printer *buffer, gassign *gs, int spc,
break;
case ABS_EXPR:
+ case ABSU_EXPR:
if (flags & TDF_GIMPLE)
{
- pp_string (buffer, "__ABS ");
+ pp_string (buffer,
+ rhs_code == ABS_EXPR ? "__ABS " : "__ABSU ");
dump_generic_node (buffer, rhs, spc, flags, false);
}
else
{
- pp_string (buffer, "ABS_EXPR <");
+ pp_string (buffer,
+ rhs_code == ABS_EXPR ? "ABS_EXPR <" : "ABSU_EXPR <");
dump_generic_node (buffer, rhs, spc, flags, false);
pp_greater (buffer);
}
diff --git a/gcc/gimple-ssa-backprop.c b/gcc/gimple-ssa-backprop.c
index 27aa575..a38b5eb 100644
--- a/gcc/gimple-ssa-backprop.c
+++ b/gcc/gimple-ssa-backprop.c
@@ -405,6 +405,7 @@ backprop::process_assign_use (gassign *assign, tree rhs, usage_info *info)
switch (gimple_assign_rhs_code (assign))
{
case ABS_EXPR:
+ case ABSU_EXPR:
/* The sign of the input doesn't matter. */
info->flags.ignore_sign = true;
break;
@@ -681,6 +682,7 @@ strip_sign_op_1 (tree rhs)
switch (gimple_assign_rhs_code (assign))
{
case ABS_EXPR:
+ case ABSU_EXPR:
case NEGATE_EXPR:
return gimple_assign_rhs1 (assign);
diff --git a/gcc/match.pd b/gcc/match.pd
index 7033730..ba52bb0 100644
--- a/gcc/match.pd
+++ b/gcc/match.pd
@@ -90,6 +90,17 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
(match (nop_convert @0)
@0)
+/* Transform likes of (char) ABS_EXPR <(int) x> into (char) ABSU_EXPR <x>
+ ABSU_EXPR returns unsigned absolute value of the operand and the operand
+ of the ABSU_EXPR will have the corresponding signed type. */
+(simplify (abs (convert @0))
+ (if (ANY_INTEGRAL_TYPE_P (TREE_TYPE (@0))
+ && !TYPE_UNSIGNED (TREE_TYPE (@0))
+ && element_precision (type) > element_precision (TREE_TYPE (@0)))
+ (with { tree utype = unsigned_type_for (TREE_TYPE (@0)); }
+ (convert (absu:utype @0)))))
+
+
/* Simplifications of operations with one constant operand and
simplifications to constants or single values. */
diff --git a/gcc/optabs-tree.c b/gcc/optabs-tree.c
index 71e172c..aa119ec 100644
--- a/gcc/optabs-tree.c
+++ b/gcc/optabs-tree.c
@@ -237,6 +237,8 @@ optab_for_tree_code (enum tree_code code, const_tree type,
case ABS_EXPR:
return trapv ? absv_optab : abs_optab;
+ case ABSU_EXPR:
+ return abs_optab;
default:
return unknown_optab;
}
diff --git a/gcc/testsuite/gcc.dg/absu.c b/gcc/testsuite/gcc.dg/absu.c
index e69de29..063da28 100644
--- a/gcc/testsuite/gcc.dg/absu.c
+++ b/gcc/testsuite/gcc.dg/absu.c
@@ -0,0 +1,85 @@
+
+/* { dg-do run } */
+/* { dg-options "-O0" } */
+
+#include <limits.h>
+#define ABS(x) (((x) >= 0) ? (x) : -(x))
+
+#define DEF_TEST(TYPE) \
+void foo_##TYPE (signed TYPE x, unsigned TYPE y){ \
+ TYPE t = ABS (x); \
+ if (t != y) \
+ __builtin_abort (); \
+} \
+
+DEF_TEST (char);
+DEF_TEST (short);
+DEF_TEST (int);
+DEF_TEST (long);
+
+int main ()
+{
+ foo_char (SCHAR_MIN + 1, SCHAR_MAX);
+ foo_char (0, 0);
+ foo_char (-1, 1);
+ foo_char (1, 1);
+ foo_char (SCHAR_MAX, SCHAR_MAX);
+
+ foo_int (-1, 1);
+ foo_int (0, 0);
+ foo_int (INT_MAX, INT_MAX);
+ foo_int (INT_MIN + 1, INT_MAX);
+
+ foo_short (-1, 1);
+ foo_short (0, 0);
+ foo_short (SHRT_MAX, SHRT_MAX);
+ foo_short (SHRT_MIN + 1, SHRT_MAX);
+
+ foo_long (-1, 1);
+ foo_long (0, 0);
+ foo_long (LONG_MAX, LONG_MAX);
+ foo_long (LONG_MIN + 1, LONG_MAX);
+
+ return 0;
+}
+
+/* { dg-do run } */
+/* { dg-options "-O0" } */
+
+#include <limits.h>
+#define ABS(x) (((x) >= 0) ? (x) : -(x))
+
+#define DEF_TEST(TYPE) \
+void foo_##TYPE (signed TYPE x, unsigned TYPE y){ \
+ TYPE t = ABS (x); \
+ if (t != y) \
+ __builtin_abort (); \
+} \
+
+DEF_TEST (char);
+DEF_TEST (short);
+DEF_TEST (int);
+DEF_TEST (long);
+void main ()
+{
+ foo_char (SCHAR_MIN + 1, SCHAR_MAX);
+ foo_char (0, 0);
+ foo_char (-1, 1);
+ foo_char (1, 1);
+ foo_char (SCHAR_MAX, SCHAR_MAX);
+
+ foo_int (-1, 1);
+ foo_int (0, 0);
+ foo_int (INT_MAX, INT_MAX);
+ foo_int (INT_MIN + 1, INT_MAX);
+
+ foo_short (-1, 1);
+ foo_short (0, 0);
+ foo_short (SHRT_MAX, SHRT_MAX);
+ foo_short (SHRT_MIN + 1, SHRT_MAX);
+
+ foo_long (-1, 1);
+ foo_long (0, 0);
+ foo_long (LONG_MAX, LONG_MAX);
+ foo_long (LONG_MIN + 1, LONG_MAX);
+}
diff --git a/gcc/testsuite/gcc.dg/gimplefe-29.c b/gcc/testsuite/gcc.dg/gimplefe-29.c
index e69de29..54b86ef 100644
--- a/gcc/testsuite/gcc.dg/gimplefe-29.c
+++ b/gcc/testsuite/gcc.dg/gimplefe-29.c
@@ -0,0 +1,11 @@
+/* { dg-do compile } */
+/* { dg-options "-O -fgimple -fdump-tree-ssa-gimple" } */
+
+unsigned int __GIMPLE() f(int a)
+{
+ unsigned int t0;
+ t0_1 = __ABSU a;
+ return t0_1;
+}
+
+/* { dg-final { scan-tree-dump "__ABSU a" "ssa" } } */
diff --git a/gcc/testsuite/gcc.target/aarch64/pr64946.c b/gcc/testsuite/gcc.target/aarch64/pr64946.c
index e69de29..736656f 100644
--- a/gcc/testsuite/gcc.target/aarch64/pr64946.c
+++ b/gcc/testsuite/gcc.target/aarch64/pr64946.c
@@ -0,0 +1,13 @@
+
+/* { dg-do compile } */
+/* { dg-options "-O3" } */
+
+signed char a[100],b[100];
+void absolute_s8 (void)
+{
+ int i;
+ for (i=0; i<16; i++)
+ a[i] = (b[i] > 0 ? b[i] : -b[i]);
+};
+
+/* { dg-final { scan-assembler-times "abs\tv\[0-9\]+.16b, v\[0-9\]+.16b" 1 } } */
diff --git a/gcc/tree-cfg.c b/gcc/tree-cfg.c
index 7f48d2d..6df42cc 100644
--- a/gcc/tree-cfg.c
+++ b/gcc/tree-cfg.c
@@ -3685,6 +3685,12 @@ verify_gimple_assign_unary (gassign *stmt)
case PAREN_EXPR:
case CONJ_EXPR:
break;
+ case ABSU_EXPR:
+ if (!TYPE_UNSIGNED (lhs_type)
+ || !ANY_INTEGRAL_TYPE_P (rhs1_type))
+ return true;
+ return false;
+ break;
case VEC_DUPLICATE_EXPR:
if (TREE_CODE (lhs_type) != VECTOR_TYPE
diff --git a/gcc/tree-eh.c b/gcc/tree-eh.c
index 3609bca..da87466 100644
--- a/gcc/tree-eh.c
+++ b/gcc/tree-eh.c
@@ -2471,6 +2471,10 @@ operation_could_trap_helper_p (enum tree_code op,
return true;
return false;
+ case ABSU_EXPR:
+ /* ABSU_EXPR never traps. */
+ return false;
+
case PLUS_EXPR:
case MINUS_EXPR:
case MULT_EXPR:
diff --git a/gcc/tree-inline.c b/gcc/tree-inline.c
index 5a0a252..d272974 100644
--- a/gcc/tree-inline.c
+++ b/gcc/tree-inline.c
@@ -3866,6 +3866,7 @@ estimate_operator_cost (enum tree_code code, eni_weights *weights,
case MIN_EXPR:
case MAX_EXPR:
case ABS_EXPR:
+ case ABSU_EXPR:
case LSHIFT_EXPR:
case RSHIFT_EXPR:
diff --git a/gcc/tree-pretty-print.c b/gcc/tree-pretty-print.c
index bc36c28..612a18f 100644
--- a/gcc/tree-pretty-print.c
+++ b/gcc/tree-pretty-print.c
@@ -2463,6 +2463,12 @@ dump_generic_node (pretty_printer *pp, tree node, int spc, dump_flags_t flags,
pp_greater (pp);
break;
+ case ABSU_EXPR:
+ pp_string (pp, "ABSU_EXPR <");
+ dump_generic_node (pp, TREE_OPERAND (node, 0), spc, flags, false);
+ pp_greater (pp);
+ break;
+
case RANGE_EXPR:
NIY;
break;
diff --git a/gcc/tree-vect-patterns.c b/gcc/tree-vect-patterns.c
index 5c2578f..e4df6c8 100644
--- a/gcc/tree-vect-patterns.c
+++ b/gcc/tree-vect-patterns.c
@@ -614,7 +614,8 @@ vect_recog_sad_pattern (vec<gimple *> *stmts, tree *type_in,
gcc_assert (abs_stmt_vinfo);
if (STMT_VINFO_DEF_TYPE (abs_stmt_vinfo) != vect_internal_def)
return NULL;
- if (gimple_assign_rhs_code (abs_stmt) != ABS_EXPR)
+ if (gimple_assign_rhs_code (abs_stmt) != ABS_EXPR
+ && gimple_assign_rhs_code (abs_stmt) != ABSU_EXPR)
return NULL;
tree abs_oprnd = gimple_assign_rhs1 (abs_stmt);
diff --git a/gcc/tree-vect-stmts.c b/gcc/tree-vect-stmts.c
index 4539f6a..c71b688 100644
--- a/gcc/tree-vect-stmts.c
+++ b/gcc/tree-vect-stmts.c
@@ -5995,7 +5995,10 @@ vectorizable_operation (gimple *stmt, gimple_stmt_iterator *gsi,
"transform binary/unary operation.\n");
/* Handle def. */
- vec_dest = vect_create_destination_var (scalar_dest, vectype);
+ if (code == ABSU_EXPR)
+ vec_dest = vect_create_destination_var (scalar_dest, vectype_out);
+ else
+ vec_dest = vect_create_destination_var (scalar_dest, vectype);
/* POINTER_DIFF_EXPR has pointer arguments which are vectorized as
vectors with unsigned elements, but the result is signed. So, we
diff --git a/gcc/tree.def b/gcc/tree.def
index 31de6c0..a1766e4 100644
--- a/gcc/tree.def
+++ b/gcc/tree.def
@@ -761,6 +761,11 @@ DEFTREECODE (MAX_EXPR, "max_expr", tcc_binary, 2)
operand of the ABS_EXPR must have the same type. */
DEFTREECODE (ABS_EXPR, "abs_expr", tcc_unary, 1)
+/* Represents the unsigned absolute value of the operand.
+ An ABSU_EXPR must have unsigned INTEGER_TYPE. The operand of the ABSU_EXPR
+ must have the corresponding signed type. */
+DEFTREECODE (ABSU_EXPR, "absu_expr", tcc_unary, 1)
+
/* Shift operations for shift and rotate.
Shift means logical shift if done on an
unsigned type, arithmetic shift if done on a signed type.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFC][PR64946] "abs" vectorization fails for char/short types
2018-06-04 8:18 ` Kugan Vivekanandarajah
@ 2018-06-04 8:39 ` Richard Biener
2018-06-11 8:28 ` Kugan Vivekanandarajah
0 siblings, 1 reply; 11+ messages in thread
From: Richard Biener @ 2018-06-04 8:39 UTC (permalink / raw)
To: Kugan Vivekanandarajah; +Cc: Andrew Pinski, GCC Patches
On Mon, Jun 4, 2018 at 10:18 AM Kugan Vivekanandarajah
<kugan.vivekanandarajah@linaro.org> wrote:
>
> Hi Richard,
>
> Thanks for the review.
>
> On 1 June 2018 at 22:20, Richard Biener <richard.guenther@gmail.com> wrote:
> > On Fri, Jun 1, 2018 at 4:12 AM Kugan Vivekanandarajah
> > <kugan.vivekanandarajah@linaro.org> wrote:
> >>
> >> Hi Richard,
> >>
> >> This is the revised patch based on the review and the discussion in
> >> https://gcc.gnu.org/ml/gcc/2018-05/msg00179.html.
> >>
> >> In summary:
> >> - I skipped (element_precision (type) < element_precision (TREE_TYPE
> >> (@0))) in the match.pd pattern as this would prevent transformation
> >> for the case in PR.
> >> that is, I am interested in is something like:
> >> char t = (char) ABS_EXPR <(int) x>
> >> and I want to generate
> >> char t = (char) ABSU_EXPR <x>
> >>
> >> - I also haven't added all the necessary match.pd changes for
> >> ABSU_EXPR. I have a patch for that but will submit separately based on
> >> this reveiw.
> >>
> >> - I also tried to add ABSU_EXPRsupport in the places as necessary by
> >> grepping for ABS_EXPR.
> >>
> >> - I also had to add special casing in vectorizer for ABSU_EXP as its
> >> result is unsigned type.
> >>
> >> Is this OK. Patch bootstraps and the regression test is ongoing.
> >
> > The c/c-typeck.c:build_unary_op change looks unnecessary - the
> > C FE should never generate this directly (the c-common one might
> > be triggered by early folding I guess).
>
> The Gimple FE testcase is running into this.
Ah, OK then.
> >
> > @@ -1761,6 +1762,9 @@ const_unop (enum tree_code code, tree type, tree arg0)
> > if (TREE_CODE (arg0) == INTEGER_CST || TREE_CODE (arg0) == REAL_CST)
> > return fold_abs_const (arg0, type);
> > break;
> > + case ABSU_EXPR:
> > + return fold_convert (type, fold_abs_const (arg0,
> > + signed_type_for (type)));
> >
> > case CONJ_EXPR:
> >
> > I think this will get you bogus TREE_OVERFLOW flags set on ABSU (-INT_MIN).
> >
> > I think you want to change fold_abs_const to properly deal with arg0 being
> > signed and type unsigned. That is, sth like
> >
> > diff --git a/gcc/fold-const.c b/gcc/fold-const.c
> > index 6f80f1b1d69..f60f9c77e91 100644
> > --- a/gcc/fold-const.c
> > +++ b/gcc/fold-const.c
> > @@ -13843,18 +13843,19 @@ fold_abs_const (tree arg0, tree type)
> > {
> > /* If the value is unsigned or non-negative, then the absolute value
> > is the same as the ordinary value. */
> > - if (!wi::neg_p (wi::to_wide (arg0), TYPE_SIGN (type)))
> > - t = arg0;
> > + wide_int val = wi::to_wide (arg0);
> > + bool overflow = false;
> > + if (!wi::neg_p (val, TYPE_SIGN (TREE_TYPE (arg0))))
> > + ;
> >
> > /* If the value is negative, then the absolute value is
> > its negation. */
> > else
> > - {
> > - bool overflow;
> > - wide_int val = wi::neg (wi::to_wide (arg0), &overflow);
> > - t = force_fit_type (type, val, -1,
> > - overflow | TREE_OVERFLOW (arg0));
> > - }
> > + wide_int val = wi::neg (val, &overflow);
> > +
> > + /* Force to the destination type, set TREE_OVERFLOW for signed
> > + TYPE only. */
> > + t = force_fit_type (type, val, 1, overflow | TREE_OVERFLOW (arg0));
> > }
> > break;
> >
> > and then simply share the const_unop code with ABS_EXPR.
>
> Done.
>
> > diff --git a/gcc/match.pd b/gcc/match.pd
> > index 14386da..7d7c132 100644
> > --- a/gcc/match.pd
> > +++ b/gcc/match.pd
> > @@ -102,6 +102,14 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
> > (match (nop_convert @0)
> > @0)
> >
> > +(simplify (abs (convert @0))
> > + (if (ANY_INTEGRAL_TYPE_P (TREE_TYPE (@0))
> > + && !TYPE_UNSIGNED (TREE_TYPE (@0))
> > + && !TYPE_UNSIGNED (type))
> > + (with { tree utype = unsigned_type_for (TREE_TYPE (@0)); }
> > + (convert (absu:utype @0)))))
> > +
> > +
> >
> > please put a comment before the pattern. I believe there's no
> > need to check for !TYPE_UNSIGNED (type). Note this
> > also converts abs ((char)int-var) to (char)absu(int-var) which
> > doesn't make sense. The original issue you want to address
> > here is the case where TYPE_PRECISION of @0 is less than
> > the precision of type. That is, you want to remove language
> > introduced integer promotion of @0 which only is possible
> > with ABSU. So please do add such precision check
> > (I simply suggested the bogus direction of the test).
>
> Done.
> >
> > diff --git a/gcc/tree-cfg.c b/gcc/tree-cfg.c
> > index 68f4fd3..9b62583 100644
> > --- a/gcc/tree-cfg.c
> > +++ b/gcc/tree-cfg.c
> > @@ -3685,6 +3685,12 @@ verify_gimple_assign_unary (gassign *stmt)
> > case PAREN_EXPR:
> > case CONJ_EXPR:
> > break;
> > + case ABSU_EXPR:
> > + if (!TYPE_UNSIGNED (lhs_type)
> > + || !ANY_INTEGRAL_TYPE_P (rhs1_type))
> >
> > if (!ANY_INTEGRAL_TYPE_P (lhs_type)
> > || !TYPE_UNSIGNED (lhs_type)
> > || !ANY_INTEGRAL_TYPE_P (rhs1_type)
> > || TYPE_UNSIGNED (rhs1_type)
> > || element_precision (lhs_type) != element_precision (rhs1_type))
> > {
> > error ("invalid types for ABSU_EXPR");
> > debug_generic_expr (lhs_type);
> > debug_generic_expr (rhs1_type);
> > return true;
> > }
> >
^^^ you forgot this one.
> > + return true;
> > + return false;
> > + break;
> >
> > diff --git a/gcc/tree-eh.c b/gcc/tree-eh.c
> > index 30c6d9e..44b1399 100644
> > --- a/gcc/tree-eh.c
> > +++ b/gcc/tree-eh.c
> > @@ -2465,6 +2465,7 @@ operation_could_trap_helper_p (enum tree_code op,
> >
> > case NEGATE_EXPR:
> > case ABS_EXPR:
> > + case ABSU_EXPR:
> > case CONJ_EXPR:
> > /* These operations don't trap with floating point. */
> > if (honor_trapv)
> >
> > ABSU never traps. Please instead unconditionally return false.
> Done.
>
> >
> > diff --git a/gcc/tree-vect-stmts.c b/gcc/tree-vect-stmts.c
> > index 66c78de..b52d714 100644
> > --- a/gcc/tree-vect-stmts.c
> > +++ b/gcc/tree-vect-stmts.c
> > @@ -5995,7 +5995,11 @@ vectorizable_operation (gimple *stmt,
> > gimple_stmt_iterator *gsi,
> > "transform binary/unary operation.\n");
> >
> > /* Handle def. */
> > - vec_dest = vect_create_destination_var (scalar_dest, vectype);
> > + if (code == ABSU_EXPR)
> > + vec_dest = vect_create_destination_var (scalar_dest,
> > + unsigned_type_for (vectype));
> > + else
> > + vec_dest = vect_create_destination_var (scalar_dest, vectype);
> >
> > /* POINTER_DIFF_EXPR has pointer arguments which are vectorized as
> > vectors with unsigned elements, but the result is signed. So, we
> >
> > simply use vectype_out for creation of vec_dest.
> Done.
/* Handle def. */
- vec_dest = vect_create_destination_var (scalar_dest, vectype);
+ if (code == ABSU_EXPR)
+ vec_dest = vect_create_destination_var (scalar_dest, vectype_out);
+ else
+ vec_dest = vect_create_destination_var (scalar_dest, vectype);
I meant _always_ vectype_out. Thus unconditionally
vec_dest = vect_create_destination_var (scalar_dest, vectype_out);
OK with those two changes.
Thanks,
Richard.
> >
> > diff --git a/gcc/tree.def b/gcc/tree.def
> > index c660b2c..5fec781 100644
> > --- a/gcc/tree.def
> > +++ b/gcc/tree.def
> > @@ -760,6 +760,7 @@ DEFTREECODE (MAX_EXPR, "max_expr", tcc_binary, 2)
> > An ABS_EXPR must have either an INTEGER_TYPE or a REAL_TYPE. The
> > operand of the ABS_EXPR must have the same type. */
> > DEFTREECODE (ABS_EXPR, "abs_expr", tcc_unary, 1)
> > +DEFTREECODE (ABSU_EXPR, "absu_expr", tcc_unary, 1)
> >
> > /* Shift operations for shift and rotate.
> > Shift means logical shift if done on an
> >
> > You can clearly see that the comment before ABS_EXPR doesn't apply to ABSU_EXPR
> > so please add an appropriate one. I suggest
> >
> > /* Represents the unsigned absolute value of the operand.
> > An ABSU_EXPR must have unsigned INTEGER_TYPE. The operand of the ABSU_EXPR
> > must have the corresponding signed type. */
>
> Done.
>
> Here is the reviesed patch. Is this OK?
>
> Thanks,
> Kugan
>
> >
> > Otherwise looks OK. (I didn't explicitely check for missing ABSU_EXPR
> > handling this time)
> >
> > Thanks,
> > Richard.
> >
> >
> >> Thanks,
> >> Kugan
> >>
> >>
> >> On 18 May 2018 at 12:36, Kugan Vivekanandarajah
> >> <kugan.vivekanandarajah@linaro.org> wrote:
> >> > Hi Richard,
> >> >
> >> > Thanks for the review. I am revising the patch based on Andrew's comments too.
> >> >
> >> > On 17 May 2018 at 20:36, Richard Biener <richard.guenther@gmail.com> wrote:
> >> >> On Thu, May 17, 2018 at 4:56 AM Andrew Pinski <pinskia@gmail.com> wrote:
> >> >>
> >> >>> On Wed, May 16, 2018 at 7:14 PM, Kugan Vivekanandarajah
> >> >>> <kugan.vivekanandarajah@linaro.org> wrote:
> >> >>> > As mentioned in the PR, I am trying to add ABSU_EXPR to fix this
> >> >>> > issue. In the attached patch, in fold_cond_expr_with_comparison I am
> >> >>> > generating ABSU_EXPR for these cases. As I understand, absu_expr is
> >> >>> > well defined in RTL. So, the issue is generating absu_expr and
> >> >>> > transferring to RTL in the correct way. I am not sure I am not doing
> >> >>> > all that is needed. I will clean up and add more test-cases based on
> >> >>> > the feedback.
> >> >>
> >> >>
> >> >>> diff --git a/gcc/optabs-tree.c b/gcc/optabs-tree.c
> >> >>> index 71e172c..2b812e5 100644
> >> >>> --- a/gcc/optabs-tree.c
> >> >>> +++ b/gcc/optabs-tree.c
> >> >>> @@ -235,6 +235,7 @@ optab_for_tree_code (enum tree_code code, const_tree
> >> >> type,
> >> >>> return trapv ? negv_optab : neg_optab;
> >> >>
> >> >>> case ABS_EXPR:
> >> >>> + case ABSU_EXPR:
> >> >>> return trapv ? absv_optab : abs_optab;
> >> >>
> >> >>
> >> >>> This part is not correct, it should something like this:
> >> >>
> >> >>> case ABS_EXPR:
> >> >>> return trapv ? absv_optab : abs_optab;
> >> >>> + case ABSU_EXPR:
> >> >>> + return abs_optab ;
> >> >>
> >> >>> Because ABSU is not undefined at the TYPE_MAX.
> >> >>
> >> >> Also
> >> >>
> >> >> /* Unsigned abs is simply the operand. Testing here means we don't
> >> >> risk generating incorrect code below. */
> >> >> - if (TYPE_UNSIGNED (type))
> >> >> + if (TYPE_UNSIGNED (type)
> >> >> + && (code != ABSU_EXPR))
> >> >> return op0;
> >> >>
> >> >> is wrong. ABSU of an unsigned number is still just that number.
> >> >>
> >> >> The change to fold_cond_expr_with_comparison looks odd to me
> >> >> (premature optimization). It should be done separately - it seems
> >> >> you are doing
> >> >
> >> > FE seems to be using this to generate ABS_EXPR from
> >> > c_fully_fold_internal to fold_build3_loc and so on. I changed this to
> >> > generate ABSU_EXPR for the case in the testcase. So the question
> >> > should be, in what cases do we need ABS_EXPR and in what cases do we
> >> > need ABSU_EXPR. It is not very clear to me.
> >> >
> >> >
> >> >>
> >> >> (simplify (abs (convert @0)) (convert (absu @0)))
> >> >>
> >> >> here.
> >> >>
> >> >> You touch one other place in fold-const.c but there seem to be many
> >> >> more that need ABSU_EXPR handling (you touched the one needed
> >> >> for correctness) - esp. you should at least handle constant folding
> >> >> in const_unop and the nonnegative predicate.
> >> >
> >> > OK.
> >> >>
> >> >> @@ -3167,6 +3167,9 @@ verify_expr (tree *tp, int *walk_subtrees, void *data
> >> >> ATTRIBUTE_UNUSED)
> >> >> CHECK_OP (0, "invalid operand to unary operator");
> >> >> break;
> >> >>
> >> >> + case ABSU_EXPR:
> >> >> + break;
> >> >> +
> >> >> case REALPART_EXPR:
> >> >> case IMAGPART_EXPR:
> >> >>
> >> >> verify_expr is no more. Did you test this recently against trunk?
> >> >
> >> > This patch is against slightly older trunk. I will rebase it.
> >> >
> >> >>
> >> >> @@ -3937,6 +3940,9 @@ verify_gimple_assign_unary (gassign *stmt)
> >> >> case PAREN_EXPR:
> >> >> case CONJ_EXPR:
> >> >> break;
> >> >> + case ABSU_EXPR:
> >> >> + /* FIXME. */
> >> >> + return false;
> >> >>
> >> >> no - please not! Please add verification here - ABSU should be only
> >> >> called on INTEGRAL, vector or complex INTEGRAL types and the
> >> >> type of the LHS should be always the unsigned variant of the
> >> >> argument type.
> >> >
> >> > OK.
> >> >>
> >> >> if (is_gimple_val (cond_expr))
> >> >> return cond_expr;
> >> >>
> >> >> - if (TREE_CODE (cond_expr) == ABS_EXPR)
> >> >> + if (TREE_CODE (cond_expr) == ABS_EXPR
> >> >> + || TREE_CODE (cond_expr) == ABSU_EXPR)
> >> >> {
> >> >> rhs1 = TREE_OPERAND (cond_expr, 1);
> >> >> STRIP_USELESS_TYPE_CONVERSION (rhs1);
> >> >>
> >> >> err, but the next line just builds a ABS_EXPR ...
> >> >>
> >> >> How did you identify spots that need adjustment? I would expect that
> >> >> once folding generates ABSU_EXPR that you need to adjust frontends
> >> >> (C++ constexpr handling for example). Also I miss adjustments
> >> >> to gimple-pretty-print.c and the GIMPLE FE parser.
> >> >
> >> > I will add this.
> >> >>
> >> >> recursively grepping throughout the whole gcc/ tree doesn't reveal too many
> >> >> cases of ABS_EXPR so I think it's reasonable to audit all of them.
> >> >>
> >> >> I also miss some trivial absu simplifications in match.pd. There are not
> >> >> a lot of abs cases but similar ones would be good to have initially.
> >> >
> >> > I will add them in the next version.
> >> >
> >> > Thanks,
> >> > Kugan
> >> >
> >> >>
> >> >> Thanks for tackling this!
> >> >> Richard.
> >> >>
> >> >>> Thanks,
> >> >>> Andrew
> >> >>
> >> >>> >
> >> >>> > Thanks,
> >> >>> > Kugan
> >> >>> >
> >> >>> >
> >> >>> > gcc/ChangeLog:
> >> >>> >
> >> >>> > 2018-05-13 Kugan Vivekanandarajah <kugan.vivekanandarajah@linaro.org>
> >> >>> >
> >> >>> > * expr.c (expand_expr_real_2): Handle ABSU_EXPR.
> >> >>> > * fold-const.c (fold_cond_expr_with_comparison): Generate ABSU_EXPR
> >> >>> > (fold_unary_loc): Handle ABSU_EXPR.
> >> >>> > * optabs-tree.c (optab_for_tree_code): Likewise.
> >> >>> > * tree-cfg.c (verify_expr): Likewise.
> >> >>> > (verify_gimple_assign_unary): Likewise.
> >> >>> > * tree-if-conv.c (fold_build_cond_expr): Likewise.
> >> >>> > * tree-inline.c (estimate_operator_cost): Likewise.
> >> >>> > * tree-pretty-print.c (dump_generic_node): Likewise.
> >> >>> > * tree.def (ABSU_EXPR): New.
> >> >>> >
> >> >>> > gcc/testsuite/ChangeLog:
> >> >>> >
> >> >>> > 2018-05-13 Kugan Vivekanandarajah <kugan.vivekanandarajah@linaro.org>
> >> >>> >
> >> >>> > * gcc.dg/absu.c: New test.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFC][PR64946] "abs" vectorization fails for char/short types
2018-06-04 8:39 ` Richard Biener
@ 2018-06-11 8:28 ` Kugan Vivekanandarajah
2018-06-13 12:08 ` Richard Biener
0 siblings, 1 reply; 11+ messages in thread
From: Kugan Vivekanandarajah @ 2018-06-11 8:28 UTC (permalink / raw)
To: Richard Biener; +Cc: Andrew Pinski, GCC Patches
Hi Richard,
Thanks for the review and sorry for getting back to you late.
On 4 June 2018 at 18:38, Richard Biener <richard.guenther@gmail.com> wrote:
> On Mon, Jun 4, 2018 at 10:18 AM Kugan Vivekanandarajah
> <kugan.vivekanandarajah@linaro.org> wrote:
>>
>> Hi Richard,
>>
>> Thanks for the review.
>>
>> On 1 June 2018 at 22:20, Richard Biener <richard.guenther@gmail.com> wrote:
>> > On Fri, Jun 1, 2018 at 4:12 AM Kugan Vivekanandarajah
>> > <kugan.vivekanandarajah@linaro.org> wrote:
>> >>
>> >> Hi Richard,
>> >>
>> >> This is the revised patch based on the review and the discussion in
>> >> https://gcc.gnu.org/ml/gcc/2018-05/msg00179.html.
>> >>
>> >> In summary:
>> >> - I skipped (element_precision (type) < element_precision (TREE_TYPE
>> >> (@0))) in the match.pd pattern as this would prevent transformation
>> >> for the case in PR.
>> >> that is, I am interested in is something like:
>> >> char t = (char) ABS_EXPR <(int) x>
>> >> and I want to generate
>> >> char t = (char) ABSU_EXPR <x>
>> >>
>> >> - I also haven't added all the necessary match.pd changes for
>> >> ABSU_EXPR. I have a patch for that but will submit separately based on
>> >> this reveiw.
>> >>
>> >> - I also tried to add ABSU_EXPRsupport in the places as necessary by
>> >> grepping for ABS_EXPR.
>> >>
>> >> - I also had to add special casing in vectorizer for ABSU_EXP as its
>> >> result is unsigned type.
>> >>
>> >> Is this OK. Patch bootstraps and the regression test is ongoing.
>> >
>> > The c/c-typeck.c:build_unary_op change looks unnecessary - the
>> > C FE should never generate this directly (the c-common one might
>> > be triggered by early folding I guess).
>>
>> The Gimple FE testcase is running into this.
>
> Ah, OK then.
>
>> >
>> > @@ -1761,6 +1762,9 @@ const_unop (enum tree_code code, tree type, tree arg0)
>> > if (TREE_CODE (arg0) == INTEGER_CST || TREE_CODE (arg0) == REAL_CST)
>> > return fold_abs_const (arg0, type);
>> > break;
>> > + case ABSU_EXPR:
>> > + return fold_convert (type, fold_abs_const (arg0,
>> > + signed_type_for (type)));
>> >
>> > case CONJ_EXPR:
>> >
>> > I think this will get you bogus TREE_OVERFLOW flags set on ABSU (-INT_MIN).
>> >
>> > I think you want to change fold_abs_const to properly deal with arg0 being
>> > signed and type unsigned. That is, sth like
>> >
>> > diff --git a/gcc/fold-const.c b/gcc/fold-const.c
>> > index 6f80f1b1d69..f60f9c77e91 100644
>> > --- a/gcc/fold-const.c
>> > +++ b/gcc/fold-const.c
>> > @@ -13843,18 +13843,19 @@ fold_abs_const (tree arg0, tree type)
>> > {
>> > /* If the value is unsigned or non-negative, then the absolute value
>> > is the same as the ordinary value. */
>> > - if (!wi::neg_p (wi::to_wide (arg0), TYPE_SIGN (type)))
>> > - t = arg0;
>> > + wide_int val = wi::to_wide (arg0);
>> > + bool overflow = false;
>> > + if (!wi::neg_p (val, TYPE_SIGN (TREE_TYPE (arg0))))
>> > + ;
>> >
>> > /* If the value is negative, then the absolute value is
>> > its negation. */
>> > else
>> > - {
>> > - bool overflow;
>> > - wide_int val = wi::neg (wi::to_wide (arg0), &overflow);
>> > - t = force_fit_type (type, val, -1,
>> > - overflow | TREE_OVERFLOW (arg0));
>> > - }
>> > + wide_int val = wi::neg (val, &overflow);
>> > +
>> > + /* Force to the destination type, set TREE_OVERFLOW for signed
>> > + TYPE only. */
>> > + t = force_fit_type (type, val, 1, overflow | TREE_OVERFLOW (arg0));
>> > }
>> > break;
>> >
>> > and then simply share the const_unop code with ABS_EXPR.
>>
>> Done.
>>
>> > diff --git a/gcc/match.pd b/gcc/match.pd
>> > index 14386da..7d7c132 100644
>> > --- a/gcc/match.pd
>> > +++ b/gcc/match.pd
>> > @@ -102,6 +102,14 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
>> > (match (nop_convert @0)
>> > @0)
>> >
>> > +(simplify (abs (convert @0))
>> > + (if (ANY_INTEGRAL_TYPE_P (TREE_TYPE (@0))
>> > + && !TYPE_UNSIGNED (TREE_TYPE (@0))
>> > + && !TYPE_UNSIGNED (type))
>> > + (with { tree utype = unsigned_type_for (TREE_TYPE (@0)); }
>> > + (convert (absu:utype @0)))))
>> > +
>> > +
>> >
>> > please put a comment before the pattern. I believe there's no
>> > need to check for !TYPE_UNSIGNED (type). Note this
>> > also converts abs ((char)int-var) to (char)absu(int-var) which
>> > doesn't make sense. The original issue you want to address
>> > here is the case where TYPE_PRECISION of @0 is less than
>> > the precision of type. That is, you want to remove language
>> > introduced integer promotion of @0 which only is possible
>> > with ABSU. So please do add such precision check
>> > (I simply suggested the bogus direction of the test).
>>
>> Done.
>> >
>> > diff --git a/gcc/tree-cfg.c b/gcc/tree-cfg.c
>> > index 68f4fd3..9b62583 100644
>> > --- a/gcc/tree-cfg.c
>> > +++ b/gcc/tree-cfg.c
>> > @@ -3685,6 +3685,12 @@ verify_gimple_assign_unary (gassign *stmt)
>> > case PAREN_EXPR:
>> > case CONJ_EXPR:
>> > break;
>> > + case ABSU_EXPR:
>> > + if (!TYPE_UNSIGNED (lhs_type)
>> > + || !ANY_INTEGRAL_TYPE_P (rhs1_type))
>> >
>> > if (!ANY_INTEGRAL_TYPE_P (lhs_type)
>> > || !TYPE_UNSIGNED (lhs_type)
>> > || !ANY_INTEGRAL_TYPE_P (rhs1_type)
>> > || TYPE_UNSIGNED (rhs1_type)
>> > || element_precision (lhs_type) != element_precision (rhs1_type))
>> > {
>> > error ("invalid types for ABSU_EXPR");
>> > debug_generic_expr (lhs_type);
>> > debug_generic_expr (rhs1_type);
>> > return true;
>> > }
>> >
>
> ^^^ you forgot this one.
>
>
>> > + return true;
>> > + return false;
>> > + break;
>> >
>> > diff --git a/gcc/tree-eh.c b/gcc/tree-eh.c
>> > index 30c6d9e..44b1399 100644
>> > --- a/gcc/tree-eh.c
>> > +++ b/gcc/tree-eh.c
>> > @@ -2465,6 +2465,7 @@ operation_could_trap_helper_p (enum tree_code op,
>> >
>> > case NEGATE_EXPR:
>> > case ABS_EXPR:
>> > + case ABSU_EXPR:
>> > case CONJ_EXPR:
>> > /* These operations don't trap with floating point. */
>> > if (honor_trapv)
>> >
>> > ABSU never traps. Please instead unconditionally return false.
>> Done.
>>
>> >
>> > diff --git a/gcc/tree-vect-stmts.c b/gcc/tree-vect-stmts.c
>> > index 66c78de..b52d714 100644
>> > --- a/gcc/tree-vect-stmts.c
>> > +++ b/gcc/tree-vect-stmts.c
>> > @@ -5995,7 +5995,11 @@ vectorizable_operation (gimple *stmt,
>> > gimple_stmt_iterator *gsi,
>> > "transform binary/unary operation.\n");
>> >
>> > /* Handle def. */
>> > - vec_dest = vect_create_destination_var (scalar_dest, vectype);
>> > + if (code == ABSU_EXPR)
>> > + vec_dest = vect_create_destination_var (scalar_dest,
>> > + unsigned_type_for (vectype));
>> > + else
>> > + vec_dest = vect_create_destination_var (scalar_dest, vectype);
>> >
>> > /* POINTER_DIFF_EXPR has pointer arguments which are vectorized as
>> > vectors with unsigned elements, but the result is signed. So, we
>> >
>> > simply use vectype_out for creation of vec_dest.
>> Done.
>
> /* Handle def. */
> - vec_dest = vect_create_destination_var (scalar_dest, vectype);
> + if (code == ABSU_EXPR)
> + vec_dest = vect_create_destination_var (scalar_dest, vectype_out);
> + else
> + vec_dest = vect_create_destination_var (scalar_dest, vectype);
>
> I meant _always_ vectype_out. Thus unconditionally
>
> vec_dest = vect_create_destination_var (scalar_dest, vectype_out);
Some testcases are failing with the changes.
gcc:gcc.dg/tree-ssa/pr83329.c (internal compiler error) : This seems
to be due to POINTER_DIFF_EXPR (?)
There is also
gcc:gcc.target/i386/avx2-vshift-1.c (internal compiler error)
gcc:gcc.target/i386/xop-vshift-1.c (internal compiler error)
Example:
/home/tcwg-buildslave/workspace/tcwg-buildfarm_2/tcwg-x86_32-build/snapshots/gcc.git~master_rev_bfa6b77/gcc/testsuite/gcc.target/i386/xop-vshift-1.c:73:1:
error: type mismatch in binary expression^M
vector(4) long long unsigned int^M
^M
vector(4) long long int^M
^M
vector(4) long long int^M
^M
vect_patt_17.54_21 = vect__2.53_16 & vect_cst__20;^M
during GIMPLE pass: vect^M
Thanks,
Kugan
>
> OK with those two changes.
>
> Thanks,
> Richard.
>
>> >
>> > diff --git a/gcc/tree.def b/gcc/tree.def
>> > index c660b2c..5fec781 100644
>> > --- a/gcc/tree.def
>> > +++ b/gcc/tree.def
>> > @@ -760,6 +760,7 @@ DEFTREECODE (MAX_EXPR, "max_expr", tcc_binary, 2)
>> > An ABS_EXPR must have either an INTEGER_TYPE or a REAL_TYPE. The
>> > operand of the ABS_EXPR must have the same type. */
>> > DEFTREECODE (ABS_EXPR, "abs_expr", tcc_unary, 1)
>> > +DEFTREECODE (ABSU_EXPR, "absu_expr", tcc_unary, 1)
>> >
>> > /* Shift operations for shift and rotate.
>> > Shift means logical shift if done on an
>> >
>> > You can clearly see that the comment before ABS_EXPR doesn't apply to ABSU_EXPR
>> > so please add an appropriate one. I suggest
>> >
>> > /* Represents the unsigned absolute value of the operand.
>> > An ABSU_EXPR must have unsigned INTEGER_TYPE. The operand of the ABSU_EXPR
>> > must have the corresponding signed type. */
>>
>> Done.
>>
>> Here is the reviesed patch. Is this OK?
>>
>> Thanks,
>> Kugan
>>
>> >
>> > Otherwise looks OK. (I didn't explicitely check for missing ABSU_EXPR
>> > handling this time)
>> >
>> > Thanks,
>> > Richard.
>> >
>> >
>> >> Thanks,
>> >> Kugan
>> >>
>> >>
>> >> On 18 May 2018 at 12:36, Kugan Vivekanandarajah
>> >> <kugan.vivekanandarajah@linaro.org> wrote:
>> >> > Hi Richard,
>> >> >
>> >> > Thanks for the review. I am revising the patch based on Andrew's comments too.
>> >> >
>> >> > On 17 May 2018 at 20:36, Richard Biener <richard.guenther@gmail.com> wrote:
>> >> >> On Thu, May 17, 2018 at 4:56 AM Andrew Pinski <pinskia@gmail.com> wrote:
>> >> >>
>> >> >>> On Wed, May 16, 2018 at 7:14 PM, Kugan Vivekanandarajah
>> >> >>> <kugan.vivekanandarajah@linaro.org> wrote:
>> >> >>> > As mentioned in the PR, I am trying to add ABSU_EXPR to fix this
>> >> >>> > issue. In the attached patch, in fold_cond_expr_with_comparison I am
>> >> >>> > generating ABSU_EXPR for these cases. As I understand, absu_expr is
>> >> >>> > well defined in RTL. So, the issue is generating absu_expr and
>> >> >>> > transferring to RTL in the correct way. I am not sure I am not doing
>> >> >>> > all that is needed. I will clean up and add more test-cases based on
>> >> >>> > the feedback.
>> >> >>
>> >> >>
>> >> >>> diff --git a/gcc/optabs-tree.c b/gcc/optabs-tree.c
>> >> >>> index 71e172c..2b812e5 100644
>> >> >>> --- a/gcc/optabs-tree.c
>> >> >>> +++ b/gcc/optabs-tree.c
>> >> >>> @@ -235,6 +235,7 @@ optab_for_tree_code (enum tree_code code, const_tree
>> >> >> type,
>> >> >>> return trapv ? negv_optab : neg_optab;
>> >> >>
>> >> >>> case ABS_EXPR:
>> >> >>> + case ABSU_EXPR:
>> >> >>> return trapv ? absv_optab : abs_optab;
>> >> >>
>> >> >>
>> >> >>> This part is not correct, it should something like this:
>> >> >>
>> >> >>> case ABS_EXPR:
>> >> >>> return trapv ? absv_optab : abs_optab;
>> >> >>> + case ABSU_EXPR:
>> >> >>> + return abs_optab ;
>> >> >>
>> >> >>> Because ABSU is not undefined at the TYPE_MAX.
>> >> >>
>> >> >> Also
>> >> >>
>> >> >> /* Unsigned abs is simply the operand. Testing here means we don't
>> >> >> risk generating incorrect code below. */
>> >> >> - if (TYPE_UNSIGNED (type))
>> >> >> + if (TYPE_UNSIGNED (type)
>> >> >> + && (code != ABSU_EXPR))
>> >> >> return op0;
>> >> >>
>> >> >> is wrong. ABSU of an unsigned number is still just that number.
>> >> >>
>> >> >> The change to fold_cond_expr_with_comparison looks odd to me
>> >> >> (premature optimization). It should be done separately - it seems
>> >> >> you are doing
>> >> >
>> >> > FE seems to be using this to generate ABS_EXPR from
>> >> > c_fully_fold_internal to fold_build3_loc and so on. I changed this to
>> >> > generate ABSU_EXPR for the case in the testcase. So the question
>> >> > should be, in what cases do we need ABS_EXPR and in what cases do we
>> >> > need ABSU_EXPR. It is not very clear to me.
>> >> >
>> >> >
>> >> >>
>> >> >> (simplify (abs (convert @0)) (convert (absu @0)))
>> >> >>
>> >> >> here.
>> >> >>
>> >> >> You touch one other place in fold-const.c but there seem to be many
>> >> >> more that need ABSU_EXPR handling (you touched the one needed
>> >> >> for correctness) - esp. you should at least handle constant folding
>> >> >> in const_unop and the nonnegative predicate.
>> >> >
>> >> > OK.
>> >> >>
>> >> >> @@ -3167,6 +3167,9 @@ verify_expr (tree *tp, int *walk_subtrees, void *data
>> >> >> ATTRIBUTE_UNUSED)
>> >> >> CHECK_OP (0, "invalid operand to unary operator");
>> >> >> break;
>> >> >>
>> >> >> + case ABSU_EXPR:
>> >> >> + break;
>> >> >> +
>> >> >> case REALPART_EXPR:
>> >> >> case IMAGPART_EXPR:
>> >> >>
>> >> >> verify_expr is no more. Did you test this recently against trunk?
>> >> >
>> >> > This patch is against slightly older trunk. I will rebase it.
>> >> >
>> >> >>
>> >> >> @@ -3937,6 +3940,9 @@ verify_gimple_assign_unary (gassign *stmt)
>> >> >> case PAREN_EXPR:
>> >> >> case CONJ_EXPR:
>> >> >> break;
>> >> >> + case ABSU_EXPR:
>> >> >> + /* FIXME. */
>> >> >> + return false;
>> >> >>
>> >> >> no - please not! Please add verification here - ABSU should be only
>> >> >> called on INTEGRAL, vector or complex INTEGRAL types and the
>> >> >> type of the LHS should be always the unsigned variant of the
>> >> >> argument type.
>> >> >
>> >> > OK.
>> >> >>
>> >> >> if (is_gimple_val (cond_expr))
>> >> >> return cond_expr;
>> >> >>
>> >> >> - if (TREE_CODE (cond_expr) == ABS_EXPR)
>> >> >> + if (TREE_CODE (cond_expr) == ABS_EXPR
>> >> >> + || TREE_CODE (cond_expr) == ABSU_EXPR)
>> >> >> {
>> >> >> rhs1 = TREE_OPERAND (cond_expr, 1);
>> >> >> STRIP_USELESS_TYPE_CONVERSION (rhs1);
>> >> >>
>> >> >> err, but the next line just builds a ABS_EXPR ...
>> >> >>
>> >> >> How did you identify spots that need adjustment? I would expect that
>> >> >> once folding generates ABSU_EXPR that you need to adjust frontends
>> >> >> (C++ constexpr handling for example). Also I miss adjustments
>> >> >> to gimple-pretty-print.c and the GIMPLE FE parser.
>> >> >
>> >> > I will add this.
>> >> >>
>> >> >> recursively grepping throughout the whole gcc/ tree doesn't reveal too many
>> >> >> cases of ABS_EXPR so I think it's reasonable to audit all of them.
>> >> >>
>> >> >> I also miss some trivial absu simplifications in match.pd. There are not
>> >> >> a lot of abs cases but similar ones would be good to have initially.
>> >> >
>> >> > I will add them in the next version.
>> >> >
>> >> > Thanks,
>> >> > Kugan
>> >> >
>> >> >>
>> >> >> Thanks for tackling this!
>> >> >> Richard.
>> >> >>
>> >> >>> Thanks,
>> >> >>> Andrew
>> >> >>
>> >> >>> >
>> >> >>> > Thanks,
>> >> >>> > Kugan
>> >> >>> >
>> >> >>> >
>> >> >>> > gcc/ChangeLog:
>> >> >>> >
>> >> >>> > 2018-05-13 Kugan Vivekanandarajah <kugan.vivekanandarajah@linaro.org>
>> >> >>> >
>> >> >>> > * expr.c (expand_expr_real_2): Handle ABSU_EXPR.
>> >> >>> > * fold-const.c (fold_cond_expr_with_comparison): Generate ABSU_EXPR
>> >> >>> > (fold_unary_loc): Handle ABSU_EXPR.
>> >> >>> > * optabs-tree.c (optab_for_tree_code): Likewise.
>> >> >>> > * tree-cfg.c (verify_expr): Likewise.
>> >> >>> > (verify_gimple_assign_unary): Likewise.
>> >> >>> > * tree-if-conv.c (fold_build_cond_expr): Likewise.
>> >> >>> > * tree-inline.c (estimate_operator_cost): Likewise.
>> >> >>> > * tree-pretty-print.c (dump_generic_node): Likewise.
>> >> >>> > * tree.def (ABSU_EXPR): New.
>> >> >>> >
>> >> >>> > gcc/testsuite/ChangeLog:
>> >> >>> >
>> >> >>> > 2018-05-13 Kugan Vivekanandarajah <kugan.vivekanandarajah@linaro.org>
>> >> >>> >
>> >> >>> > * gcc.dg/absu.c: New test.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFC][PR64946] "abs" vectorization fails for char/short types
2018-06-11 8:28 ` Kugan Vivekanandarajah
@ 2018-06-13 12:08 ` Richard Biener
0 siblings, 0 replies; 11+ messages in thread
From: Richard Biener @ 2018-06-13 12:08 UTC (permalink / raw)
To: Kugan Vivekanandarajah; +Cc: Andrew Pinski, GCC Patches
On Mon, Jun 11, 2018 at 10:28 AM Kugan Vivekanandarajah
<kugan.vivekanandarajah@linaro.org> wrote:
>
> Hi Richard,
>
> Thanks for the review and sorry for getting back to you late.
>
> On 4 June 2018 at 18:38, Richard Biener <richard.guenther@gmail.com> wrote:
> > On Mon, Jun 4, 2018 at 10:18 AM Kugan Vivekanandarajah
> > <kugan.vivekanandarajah@linaro.org> wrote:
> >>
> >> Hi Richard,
> >>
> >> Thanks for the review.
> >>
> >> On 1 June 2018 at 22:20, Richard Biener <richard.guenther@gmail.com> wrote:
> >> > On Fri, Jun 1, 2018 at 4:12 AM Kugan Vivekanandarajah
> >> > <kugan.vivekanandarajah@linaro.org> wrote:
> >> >>
> >> >> Hi Richard,
> >> >>
> >> >> This is the revised patch based on the review and the discussion in
> >> >> https://gcc.gnu.org/ml/gcc/2018-05/msg00179.html.
> >> >>
> >> >> In summary:
> >> >> - I skipped (element_precision (type) < element_precision (TREE_TYPE
> >> >> (@0))) in the match.pd pattern as this would prevent transformation
> >> >> for the case in PR.
> >> >> that is, I am interested in is something like:
> >> >> char t = (char) ABS_EXPR <(int) x>
> >> >> and I want to generate
> >> >> char t = (char) ABSU_EXPR <x>
> >> >>
> >> >> - I also haven't added all the necessary match.pd changes for
> >> >> ABSU_EXPR. I have a patch for that but will submit separately based on
> >> >> this reveiw.
> >> >>
> >> >> - I also tried to add ABSU_EXPRsupport in the places as necessary by
> >> >> grepping for ABS_EXPR.
> >> >>
> >> >> - I also had to add special casing in vectorizer for ABSU_EXP as its
> >> >> result is unsigned type.
> >> >>
> >> >> Is this OK. Patch bootstraps and the regression test is ongoing.
> >> >
> >> > The c/c-typeck.c:build_unary_op change looks unnecessary - the
> >> > C FE should never generate this directly (the c-common one might
> >> > be triggered by early folding I guess).
> >>
> >> The Gimple FE testcase is running into this.
> >
> > Ah, OK then.
> >
> >> >
> >> > @@ -1761,6 +1762,9 @@ const_unop (enum tree_code code, tree type, tree arg0)
> >> > if (TREE_CODE (arg0) == INTEGER_CST || TREE_CODE (arg0) == REAL_CST)
> >> > return fold_abs_const (arg0, type);
> >> > break;
> >> > + case ABSU_EXPR:
> >> > + return fold_convert (type, fold_abs_const (arg0,
> >> > + signed_type_for (type)));
> >> >
> >> > case CONJ_EXPR:
> >> >
> >> > I think this will get you bogus TREE_OVERFLOW flags set on ABSU (-INT_MIN).
> >> >
> >> > I think you want to change fold_abs_const to properly deal with arg0 being
> >> > signed and type unsigned. That is, sth like
> >> >
> >> > diff --git a/gcc/fold-const.c b/gcc/fold-const.c
> >> > index 6f80f1b1d69..f60f9c77e91 100644
> >> > --- a/gcc/fold-const.c
> >> > +++ b/gcc/fold-const.c
> >> > @@ -13843,18 +13843,19 @@ fold_abs_const (tree arg0, tree type)
> >> > {
> >> > /* If the value is unsigned or non-negative, then the absolute value
> >> > is the same as the ordinary value. */
> >> > - if (!wi::neg_p (wi::to_wide (arg0), TYPE_SIGN (type)))
> >> > - t = arg0;
> >> > + wide_int val = wi::to_wide (arg0);
> >> > + bool overflow = false;
> >> > + if (!wi::neg_p (val, TYPE_SIGN (TREE_TYPE (arg0))))
> >> > + ;
> >> >
> >> > /* If the value is negative, then the absolute value is
> >> > its negation. */
> >> > else
> >> > - {
> >> > - bool overflow;
> >> > - wide_int val = wi::neg (wi::to_wide (arg0), &overflow);
> >> > - t = force_fit_type (type, val, -1,
> >> > - overflow | TREE_OVERFLOW (arg0));
> >> > - }
> >> > + wide_int val = wi::neg (val, &overflow);
> >> > +
> >> > + /* Force to the destination type, set TREE_OVERFLOW for signed
> >> > + TYPE only. */
> >> > + t = force_fit_type (type, val, 1, overflow | TREE_OVERFLOW (arg0));
> >> > }
> >> > break;
> >> >
> >> > and then simply share the const_unop code with ABS_EXPR.
> >>
> >> Done.
> >>
> >> > diff --git a/gcc/match.pd b/gcc/match.pd
> >> > index 14386da..7d7c132 100644
> >> > --- a/gcc/match.pd
> >> > +++ b/gcc/match.pd
> >> > @@ -102,6 +102,14 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
> >> > (match (nop_convert @0)
> >> > @0)
> >> >
> >> > +(simplify (abs (convert @0))
> >> > + (if (ANY_INTEGRAL_TYPE_P (TREE_TYPE (@0))
> >> > + && !TYPE_UNSIGNED (TREE_TYPE (@0))
> >> > + && !TYPE_UNSIGNED (type))
> >> > + (with { tree utype = unsigned_type_for (TREE_TYPE (@0)); }
> >> > + (convert (absu:utype @0)))))
> >> > +
> >> > +
> >> >
> >> > please put a comment before the pattern. I believe there's no
> >> > need to check for !TYPE_UNSIGNED (type). Note this
> >> > also converts abs ((char)int-var) to (char)absu(int-var) which
> >> > doesn't make sense. The original issue you want to address
> >> > here is the case where TYPE_PRECISION of @0 is less than
> >> > the precision of type. That is, you want to remove language
> >> > introduced integer promotion of @0 which only is possible
> >> > with ABSU. So please do add such precision check
> >> > (I simply suggested the bogus direction of the test).
> >>
> >> Done.
> >> >
> >> > diff --git a/gcc/tree-cfg.c b/gcc/tree-cfg.c
> >> > index 68f4fd3..9b62583 100644
> >> > --- a/gcc/tree-cfg.c
> >> > +++ b/gcc/tree-cfg.c
> >> > @@ -3685,6 +3685,12 @@ verify_gimple_assign_unary (gassign *stmt)
> >> > case PAREN_EXPR:
> >> > case CONJ_EXPR:
> >> > break;
> >> > + case ABSU_EXPR:
> >> > + if (!TYPE_UNSIGNED (lhs_type)
> >> > + || !ANY_INTEGRAL_TYPE_P (rhs1_type))
> >> >
> >> > if (!ANY_INTEGRAL_TYPE_P (lhs_type)
> >> > || !TYPE_UNSIGNED (lhs_type)
> >> > || !ANY_INTEGRAL_TYPE_P (rhs1_type)
> >> > || TYPE_UNSIGNED (rhs1_type)
> >> > || element_precision (lhs_type) != element_precision (rhs1_type))
> >> > {
> >> > error ("invalid types for ABSU_EXPR");
> >> > debug_generic_expr (lhs_type);
> >> > debug_generic_expr (rhs1_type);
> >> > return true;
> >> > }
> >> >
> >
> > ^^^ you forgot this one.
> >
> >
> >> > + return true;
> >> > + return false;
> >> > + break;
> >> >
> >> > diff --git a/gcc/tree-eh.c b/gcc/tree-eh.c
> >> > index 30c6d9e..44b1399 100644
> >> > --- a/gcc/tree-eh.c
> >> > +++ b/gcc/tree-eh.c
> >> > @@ -2465,6 +2465,7 @@ operation_could_trap_helper_p (enum tree_code op,
> >> >
> >> > case NEGATE_EXPR:
> >> > case ABS_EXPR:
> >> > + case ABSU_EXPR:
> >> > case CONJ_EXPR:
> >> > /* These operations don't trap with floating point. */
> >> > if (honor_trapv)
> >> >
> >> > ABSU never traps. Please instead unconditionally return false.
> >> Done.
> >>
> >> >
> >> > diff --git a/gcc/tree-vect-stmts.c b/gcc/tree-vect-stmts.c
> >> > index 66c78de..b52d714 100644
> >> > --- a/gcc/tree-vect-stmts.c
> >> > +++ b/gcc/tree-vect-stmts.c
> >> > @@ -5995,7 +5995,11 @@ vectorizable_operation (gimple *stmt,
> >> > gimple_stmt_iterator *gsi,
> >> > "transform binary/unary operation.\n");
> >> >
> >> > /* Handle def. */
> >> > - vec_dest = vect_create_destination_var (scalar_dest, vectype);
> >> > + if (code == ABSU_EXPR)
> >> > + vec_dest = vect_create_destination_var (scalar_dest,
> >> > + unsigned_type_for (vectype));
> >> > + else
> >> > + vec_dest = vect_create_destination_var (scalar_dest, vectype);
> >> >
> >> > /* POINTER_DIFF_EXPR has pointer arguments which are vectorized as
> >> > vectors with unsigned elements, but the result is signed. So, we
> >> >
> >> > simply use vectype_out for creation of vec_dest.
> >> Done.
> >
> > /* Handle def. */
> > - vec_dest = vect_create_destination_var (scalar_dest, vectype);
> > + if (code == ABSU_EXPR)
> > + vec_dest = vect_create_destination_var (scalar_dest, vectype_out);
> > + else
> > + vec_dest = vect_create_destination_var (scalar_dest, vectype);
> >
> > I meant _always_ vectype_out. Thus unconditionally
> >
> > vec_dest = vect_create_destination_var (scalar_dest, vectype_out);
>
> Some testcases are failing with the changes.
>
> gcc:gcc.dg/tree-ssa/pr83329.c (internal compiler error) : This seems
> to be due to POINTER_DIFF_EXPR (?)
Ah, yes. I'd simply handle that in a more clear way.
> There is also
>
> gcc:gcc.target/i386/avx2-vshift-1.c (internal compiler error)
> gcc:gcc.target/i386/xop-vshift-1.c (internal compiler error)
>
> Example:
> /home/tcwg-buildslave/workspace/tcwg-buildfarm_2/tcwg-x86_32-build/snapshots/gcc.git~master_rev_bfa6b77/gcc/testsuite/gcc.target/i386/xop-vshift-1.c:73:1:
> error: type mismatch in binary expression^M
> vector(4) long long unsigned int^M
> ^M
> vector(4) long long int^M
> ^M
> vector(4) long long int^M
> ^M
> vect_patt_17.54_21 = vect__2.53_16 & vect_cst__20;^M
> during GIMPLE pass: vect^M
That one is odd and looks like a latent bug in pattern creation:
diff --git a/gcc/tree-vect-patterns.c b/gcc/tree-vect-patterns.c
index 507c5b94f07..6786ffcd4c6 100644
--- a/gcc/tree-vect-patterns.c
+++ b/gcc/tree-vect-patterns.c
@@ -2185,6 +2185,11 @@ vect_recog_vector_vector_shift_pattern
(vec<gimple *> *stmts,
TYPE_PRECISION (TREE_TYPE (oprnd1)));
def = vect_recog_temp_ssa_var (TREE_TYPE (rhs1), NULL);
def_stmt = gimple_build_assign (def, BIT_AND_EXPR, rhs1, mask);
+ stmt_vec_info new_stmt_info
+ = new_stmt_vec_info (def_stmt, vinfo);
+ set_vinfo_for_stmt (def_stmt, new_stmt_info);
+ STMT_VINFO_VECTYPE (new_stmt_info)
+ = get_vectype_for_scalar_type (TREE_TYPE (rhs1));
new_pattern_def_seq (stmt_vinfo, def_stmt);
}
}
I can see if I can make this change independently of yours (that is,
I'm bootstrapping
and testing the above together with an equivalent vectorizable_operation hunk).
Richard.
>
> Thanks,
> Kugan
>
>
>
> >
> > OK with those two changes.
> >
> > Thanks,
> > Richard.
> >
> >> >
> >> > diff --git a/gcc/tree.def b/gcc/tree.def
> >> > index c660b2c..5fec781 100644
> >> > --- a/gcc/tree.def
> >> > +++ b/gcc/tree.def
> >> > @@ -760,6 +760,7 @@ DEFTREECODE (MAX_EXPR, "max_expr", tcc_binary, 2)
> >> > An ABS_EXPR must have either an INTEGER_TYPE or a REAL_TYPE. The
> >> > operand of the ABS_EXPR must have the same type. */
> >> > DEFTREECODE (ABS_EXPR, "abs_expr", tcc_unary, 1)
> >> > +DEFTREECODE (ABSU_EXPR, "absu_expr", tcc_unary, 1)
> >> >
> >> > /* Shift operations for shift and rotate.
> >> > Shift means logical shift if done on an
> >> >
> >> > You can clearly see that the comment before ABS_EXPR doesn't apply to ABSU_EXPR
> >> > so please add an appropriate one. I suggest
> >> >
> >> > /* Represents the unsigned absolute value of the operand.
> >> > An ABSU_EXPR must have unsigned INTEGER_TYPE. The operand of the ABSU_EXPR
> >> > must have the corresponding signed type. */
> >>
> >> Done.
> >>
> >> Here is the reviesed patch. Is this OK?
> >>
> >> Thanks,
> >> Kugan
> >>
> >> >
> >> > Otherwise looks OK. (I didn't explicitely check for missing ABSU_EXPR
> >> > handling this time)
> >> >
> >> > Thanks,
> >> > Richard.
> >> >
> >> >
> >> >> Thanks,
> >> >> Kugan
> >> >>
> >> >>
> >> >> On 18 May 2018 at 12:36, Kugan Vivekanandarajah
> >> >> <kugan.vivekanandarajah@linaro.org> wrote:
> >> >> > Hi Richard,
> >> >> >
> >> >> > Thanks for the review. I am revising the patch based on Andrew's comments too.
> >> >> >
> >> >> > On 17 May 2018 at 20:36, Richard Biener <richard.guenther@gmail.com> wrote:
> >> >> >> On Thu, May 17, 2018 at 4:56 AM Andrew Pinski <pinskia@gmail.com> wrote:
> >> >> >>
> >> >> >>> On Wed, May 16, 2018 at 7:14 PM, Kugan Vivekanandarajah
> >> >> >>> <kugan.vivekanandarajah@linaro.org> wrote:
> >> >> >>> > As mentioned in the PR, I am trying to add ABSU_EXPR to fix this
> >> >> >>> > issue. In the attached patch, in fold_cond_expr_with_comparison I am
> >> >> >>> > generating ABSU_EXPR for these cases. As I understand, absu_expr is
> >> >> >>> > well defined in RTL. So, the issue is generating absu_expr and
> >> >> >>> > transferring to RTL in the correct way. I am not sure I am not doing
> >> >> >>> > all that is needed. I will clean up and add more test-cases based on
> >> >> >>> > the feedback.
> >> >> >>
> >> >> >>
> >> >> >>> diff --git a/gcc/optabs-tree.c b/gcc/optabs-tree.c
> >> >> >>> index 71e172c..2b812e5 100644
> >> >> >>> --- a/gcc/optabs-tree.c
> >> >> >>> +++ b/gcc/optabs-tree.c
> >> >> >>> @@ -235,6 +235,7 @@ optab_for_tree_code (enum tree_code code, const_tree
> >> >> >> type,
> >> >> >>> return trapv ? negv_optab : neg_optab;
> >> >> >>
> >> >> >>> case ABS_EXPR:
> >> >> >>> + case ABSU_EXPR:
> >> >> >>> return trapv ? absv_optab : abs_optab;
> >> >> >>
> >> >> >>
> >> >> >>> This part is not correct, it should something like this:
> >> >> >>
> >> >> >>> case ABS_EXPR:
> >> >> >>> return trapv ? absv_optab : abs_optab;
> >> >> >>> + case ABSU_EXPR:
> >> >> >>> + return abs_optab ;
> >> >> >>
> >> >> >>> Because ABSU is not undefined at the TYPE_MAX.
> >> >> >>
> >> >> >> Also
> >> >> >>
> >> >> >> /* Unsigned abs is simply the operand. Testing here means we don't
> >> >> >> risk generating incorrect code below. */
> >> >> >> - if (TYPE_UNSIGNED (type))
> >> >> >> + if (TYPE_UNSIGNED (type)
> >> >> >> + && (code != ABSU_EXPR))
> >> >> >> return op0;
> >> >> >>
> >> >> >> is wrong. ABSU of an unsigned number is still just that number.
> >> >> >>
> >> >> >> The change to fold_cond_expr_with_comparison looks odd to me
> >> >> >> (premature optimization). It should be done separately - it seems
> >> >> >> you are doing
> >> >> >
> >> >> > FE seems to be using this to generate ABS_EXPR from
> >> >> > c_fully_fold_internal to fold_build3_loc and so on. I changed this to
> >> >> > generate ABSU_EXPR for the case in the testcase. So the question
> >> >> > should be, in what cases do we need ABS_EXPR and in what cases do we
> >> >> > need ABSU_EXPR. It is not very clear to me.
> >> >> >
> >> >> >
> >> >> >>
> >> >> >> (simplify (abs (convert @0)) (convert (absu @0)))
> >> >> >>
> >> >> >> here.
> >> >> >>
> >> >> >> You touch one other place in fold-const.c but there seem to be many
> >> >> >> more that need ABSU_EXPR handling (you touched the one needed
> >> >> >> for correctness) - esp. you should at least handle constant folding
> >> >> >> in const_unop and the nonnegative predicate.
> >> >> >
> >> >> > OK.
> >> >> >>
> >> >> >> @@ -3167,6 +3167,9 @@ verify_expr (tree *tp, int *walk_subtrees, void *data
> >> >> >> ATTRIBUTE_UNUSED)
> >> >> >> CHECK_OP (0, "invalid operand to unary operator");
> >> >> >> break;
> >> >> >>
> >> >> >> + case ABSU_EXPR:
> >> >> >> + break;
> >> >> >> +
> >> >> >> case REALPART_EXPR:
> >> >> >> case IMAGPART_EXPR:
> >> >> >>
> >> >> >> verify_expr is no more. Did you test this recently against trunk?
> >> >> >
> >> >> > This patch is against slightly older trunk. I will rebase it.
> >> >> >
> >> >> >>
> >> >> >> @@ -3937,6 +3940,9 @@ verify_gimple_assign_unary (gassign *stmt)
> >> >> >> case PAREN_EXPR:
> >> >> >> case CONJ_EXPR:
> >> >> >> break;
> >> >> >> + case ABSU_EXPR:
> >> >> >> + /* FIXME. */
> >> >> >> + return false;
> >> >> >>
> >> >> >> no - please not! Please add verification here - ABSU should be only
> >> >> >> called on INTEGRAL, vector or complex INTEGRAL types and the
> >> >> >> type of the LHS should be always the unsigned variant of the
> >> >> >> argument type.
> >> >> >
> >> >> > OK.
> >> >> >>
> >> >> >> if (is_gimple_val (cond_expr))
> >> >> >> return cond_expr;
> >> >> >>
> >> >> >> - if (TREE_CODE (cond_expr) == ABS_EXPR)
> >> >> >> + if (TREE_CODE (cond_expr) == ABS_EXPR
> >> >> >> + || TREE_CODE (cond_expr) == ABSU_EXPR)
> >> >> >> {
> >> >> >> rhs1 = TREE_OPERAND (cond_expr, 1);
> >> >> >> STRIP_USELESS_TYPE_CONVERSION (rhs1);
> >> >> >>
> >> >> >> err, but the next line just builds a ABS_EXPR ...
> >> >> >>
> >> >> >> How did you identify spots that need adjustment? I would expect that
> >> >> >> once folding generates ABSU_EXPR that you need to adjust frontends
> >> >> >> (C++ constexpr handling for example). Also I miss adjustments
> >> >> >> to gimple-pretty-print.c and the GIMPLE FE parser.
> >> >> >
> >> >> > I will add this.
> >> >> >>
> >> >> >> recursively grepping throughout the whole gcc/ tree doesn't reveal too many
> >> >> >> cases of ABS_EXPR so I think it's reasonable to audit all of them.
> >> >> >>
> >> >> >> I also miss some trivial absu simplifications in match.pd. There are not
> >> >> >> a lot of abs cases but similar ones would be good to have initially.
> >> >> >
> >> >> > I will add them in the next version.
> >> >> >
> >> >> > Thanks,
> >> >> > Kugan
> >> >> >
> >> >> >>
> >> >> >> Thanks for tackling this!
> >> >> >> Richard.
> >> >> >>
> >> >> >>> Thanks,
> >> >> >>> Andrew
> >> >> >>
> >> >> >>> >
> >> >> >>> > Thanks,
> >> >> >>> > Kugan
> >> >> >>> >
> >> >> >>> >
> >> >> >>> > gcc/ChangeLog:
> >> >> >>> >
> >> >> >>> > 2018-05-13 Kugan Vivekanandarajah <kugan.vivekanandarajah@linaro.org>
> >> >> >>> >
> >> >> >>> > * expr.c (expand_expr_real_2): Handle ABSU_EXPR.
> >> >> >>> > * fold-const.c (fold_cond_expr_with_comparison): Generate ABSU_EXPR
> >> >> >>> > (fold_unary_loc): Handle ABSU_EXPR.
> >> >> >>> > * optabs-tree.c (optab_for_tree_code): Likewise.
> >> >> >>> > * tree-cfg.c (verify_expr): Likewise.
> >> >> >>> > (verify_gimple_assign_unary): Likewise.
> >> >> >>> > * tree-if-conv.c (fold_build_cond_expr): Likewise.
> >> >> >>> > * tree-inline.c (estimate_operator_cost): Likewise.
> >> >> >>> > * tree-pretty-print.c (dump_generic_node): Likewise.
> >> >> >>> > * tree.def (ABSU_EXPR): New.
> >> >> >>> >
> >> >> >>> > gcc/testsuite/ChangeLog:
> >> >> >>> >
> >> >> >>> > 2018-05-13 Kugan Vivekanandarajah <kugan.vivekanandarajah@linaro.org>
> >> >> >>> >
> >> >> >>> > * gcc.dg/absu.c: New test.
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2018-06-13 12:08 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-17 2:56 [RFC][PR64946] "abs" vectorization fails for char/short types Kugan Vivekanandarajah
2018-05-17 4:35 ` Andrew Pinski
2018-05-17 10:38 ` Richard Biener
2018-05-18 7:28 ` Kugan Vivekanandarajah
2018-05-18 8:01 ` Richard Biener
2018-06-01 2:12 ` Kugan Vivekanandarajah
2018-06-01 12:21 ` Richard Biener
2018-06-04 8:18 ` Kugan Vivekanandarajah
2018-06-04 8:39 ` Richard Biener
2018-06-11 8:28 ` Kugan Vivekanandarajah
2018-06-13 12:08 ` 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).