* [Patch] Fix tree-optimization/49963
@ 2011-08-17 20:05 Paolo Carlini
2011-08-17 21:52 ` Georg-Johann Lay
2011-08-18 10:10 ` Richard Guenther
0 siblings, 2 replies; 5+ messages in thread
From: Paolo Carlini @ 2011-08-17 20:05 UTC (permalink / raw)
To: gcc-patches; +Cc: Richard Guenther
[-- Attachment #1: Type: text/plain, Size: 1202 bytes --]
Hi,
I prepared the below basing on an hint provided by Joseph on the audit
trail: essentially, I'm replacing all (but two) uses of abs_hwi outside
hwint.c by absu_hwi, a variant which returns an unsigned HOST_WIDE_INT.
All the replacements make sense to me: either we are comparing two abs,
or we are passing the abs to a function actually expecting an unsigned
HOST_WIDE_INT, or we are comparing to another unsigned HOST_WIDE_INT, or
we are just comparing to a constant (I don't feel strongly in this case
but seems safe to use absu_hwi here too). I'm *not* replacing 2
occurrences in gimple_expand_builtin_pow, because those are safe anyway
in terms of range of the argument (it's an HOST_WIDE_INT / 2 or 3) and
the result is passed to a function expecting a plain HOST_WIDE_INT (ie,
gimple_expand_builtin_powi).
I sanity checked the patch on x86_64-linux and OP reported that on AVR
the patch fixes the regression.
Is it ok?
Thanks,
Paolo.
PS: compared to the draft version, the attached uses cast to unsigned in
both arms of the ? : operator, I think Joseph preferred that
stylistically in a snippet of him posted in an unrelated recent audit trail.
///////////////////////////
[-- Attachment #2: CL_49963 --]
[-- Type: text/plain, Size: 398 bytes --]
2011-08-17 Paolo Carlini <paolo.carlini@oracle.com>
Joseph Myers <joseph@codesourcery.com>
PR tree-optimization/49963
* hwint.c (absu_hwi): Define.
* hwint.h (absu_hwi): Declare.
* fold-const.c (fold_plusminus_mult_expr): Use absu_hwi instead
of abs_hwi.
* tree-ssa-math-opts.c (gimple_expand_builtin_pow): Likewise.
* tree-ssa-loop-prefetch.c (prune_ref_by_group_reuse): Likewise.
[-- Attachment #3: patch_49963 --]
[-- Type: text/plain, Size: 4736 bytes --]
Index: fold-const.c
===================================================================
--- fold-const.c (revision 177834)
+++ fold-const.c (working copy)
@@ -7036,7 +7036,7 @@ fold_plusminus_mult_expr (location_t loc, enum tre
int11 = TREE_INT_CST_LOW (arg11);
/* Move min of absolute values to int11. */
- if (abs_hwi (int01) < abs_hwi (int11))
+ if (absu_hwi (int01) < absu_hwi (int11))
{
tmp = int01, int01 = int11, int11 = tmp;
alt0 = arg00, arg00 = arg10, arg10 = alt0;
@@ -7046,7 +7046,7 @@ fold_plusminus_mult_expr (location_t loc, enum tre
else
maybe_same = arg11;
- if (exact_log2 (abs_hwi (int11)) > 0 && int01 % int11 == 0
+ if (exact_log2 (absu_hwi (int11)) > 0 && int01 % int11 == 0
/* The remainder should not be a constant, otherwise we
end up folding i * 4 + 2 to (i * 2 + 1) * 2 which has
increased the number of multiplications necessary. */
Index: tree-ssa-math-opts.c
===================================================================
--- tree-ssa-math-opts.c (revision 177834)
+++ tree-ssa-math-opts.c (working copy)
@@ -1231,7 +1231,7 @@ gimple_expand_builtin_pow (gimple_stmt_iterator *g
/* Attempt to fold powi(arg0, abs(n/2)) into multiplies. If not
possible or profitable, give up. Skip the degenerate case when
n is 1 or -1, where the result is always 1. */
- if (abs_hwi (n) != 1)
+ if (absu_hwi (n) != 1)
{
powi_x_ndiv2 = gimple_expand_builtin_powi (gsi, loc, arg0,
abs_hwi (n / 2));
@@ -1243,7 +1243,7 @@ gimple_expand_builtin_pow (gimple_stmt_iterator *g
result of the optimal multiply sequence just calculated. */
sqrt_arg0 = build_and_insert_call (gsi, loc, &target, sqrtfn, arg0);
- if (abs_hwi (n) == 1)
+ if (absu_hwi (n) == 1)
result = sqrt_arg0;
else
result = build_and_insert_binop (gsi, loc, target, MULT_EXPR,
@@ -1285,7 +1285,7 @@ gimple_expand_builtin_pow (gimple_stmt_iterator *g
/* Attempt to fold powi(arg0, abs(n/3)) into multiplies. If not
possible or profitable, give up. Skip the degenerate case when
abs(n) < 3, where the result is always 1. */
- if (abs_hwi (n) >= 3)
+ if (absu_hwi (n) >= 3)
{
powi_x_ndiv3 = gimple_expand_builtin_powi (gsi, loc, arg0,
abs_hwi (n / 3));
@@ -1298,14 +1298,14 @@ gimple_expand_builtin_pow (gimple_stmt_iterator *g
either cbrt(x) or cbrt(x) * cbrt(x). */
cbrt_x = build_and_insert_call (gsi, loc, &target, cbrtfn, arg0);
- if (abs_hwi (n) % 3 == 1)
+ if (absu_hwi (n) % 3 == 1)
powi_cbrt_x = cbrt_x;
else
powi_cbrt_x = build_and_insert_binop (gsi, loc, target, MULT_EXPR,
cbrt_x, cbrt_x);
/* Multiply the two subexpressions, unless powi(x,abs(n)/3) = 1. */
- if (abs_hwi (n) < 3)
+ if (absu_hwi (n) < 3)
result = powi_cbrt_x;
else
result = build_and_insert_binop (gsi, loc, target, MULT_EXPR,
Index: tree-ssa-loop-prefetch.c
===================================================================
--- tree-ssa-loop-prefetch.c (revision 177834)
+++ tree-ssa-loop-prefetch.c (working copy)
@@ -795,7 +795,7 @@ prune_ref_by_group_reuse (struct mem_ref *ref, str
prefetch_before = (hit_from - delta_r + step - 1) / step;
/* Do not reduce prefetch_before if we meet beyond cache size. */
- if (prefetch_before > (unsigned) abs_hwi (L2_CACHE_SIZE_BYTES / step))
+ if (prefetch_before > absu_hwi (L2_CACHE_SIZE_BYTES / step))
prefetch_before = PREFETCH_ALL;
if (prefetch_before < ref->prefetch_before)
ref->prefetch_before = prefetch_before;
Index: hwint.c
===================================================================
--- hwint.c (revision 177834)
+++ hwint.c (working copy)
@@ -109,6 +109,14 @@ abs_hwi (HOST_WIDE_INT x)
return x >= 0 ? x : -x;
}
+/* Compute the absolute value of X as an unsigned type. */
+
+unsigned HOST_WIDE_INT
+absu_hwi (HOST_WIDE_INT x)
+{
+ return x >= 0 ? (unsigned HOST_WIDE_INT)x : -(unsigned HOST_WIDE_INT)x;
+}
+
/* Compute the greatest common divisor of two numbers A and B using
Euclid's algorithm. */
Index: hwint.h
===================================================================
--- hwint.h (revision 177834)
+++ hwint.h (working copy)
@@ -233,6 +233,7 @@ exact_log2 (unsigned HOST_WIDE_INT x)
#define HOST_WIDE_INT_MAX (~(HOST_WIDE_INT_MIN))
extern HOST_WIDE_INT abs_hwi (HOST_WIDE_INT);
+extern unsigned HOST_WIDE_INT absu_hwi (HOST_WIDE_INT);
extern HOST_WIDE_INT gcd (HOST_WIDE_INT, HOST_WIDE_INT);
extern HOST_WIDE_INT pos_mul_hwi (HOST_WIDE_INT, HOST_WIDE_INT);
extern HOST_WIDE_INT mul_hwi (HOST_WIDE_INT, HOST_WIDE_INT);
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Patch] Fix tree-optimization/49963
2011-08-17 20:05 [Patch] Fix tree-optimization/49963 Paolo Carlini
@ 2011-08-17 21:52 ` Georg-Johann Lay
2011-08-18 10:10 ` Richard Guenther
1 sibling, 0 replies; 5+ messages in thread
From: Georg-Johann Lay @ 2011-08-17 21:52 UTC (permalink / raw)
To: Paolo Carlini; +Cc: gcc-patches, Richard Guenther
Paolo Carlini wrote:
>
> I sanity checked the patch on x86_64-linux and OP reported that on AVR
> the patch fixes the regression.
Not really "on" AVR; AVR are just tiny 8-bit microcontrollers ;-)
I obseved the problem when compiling for AVR on a x86-linux-gnu host
where 0x80000000 is negative.
Johann
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Patch] Fix tree-optimization/49963
2011-08-17 20:05 [Patch] Fix tree-optimization/49963 Paolo Carlini
2011-08-17 21:52 ` Georg-Johann Lay
@ 2011-08-18 10:10 ` Richard Guenther
2011-08-18 12:09 ` Paolo Carlini
1 sibling, 1 reply; 5+ messages in thread
From: Richard Guenther @ 2011-08-18 10:10 UTC (permalink / raw)
To: Paolo Carlini; +Cc: gcc-patches
On Wed, 17 Aug 2011, Paolo Carlini wrote:
> Hi,
>
> I prepared the below basing on an hint provided by Joseph on the audit trail:
> essentially, I'm replacing all (but two) uses of abs_hwi outside hwint.c by
> absu_hwi, a variant which returns an unsigned HOST_WIDE_INT. All the
> replacements make sense to me: either we are comparing two abs, or we are
> passing the abs to a function actually expecting an unsigned HOST_WIDE_INT, or
> we are comparing to another unsigned HOST_WIDE_INT, or we are just comparing
> to a constant (I don't feel strongly in this case but seems safe to use
> absu_hwi here too). I'm *not* replacing 2 occurrences in
> gimple_expand_builtin_pow, because those are safe anyway in terms of range of
> the argument (it's an HOST_WIDE_INT / 2 or 3) and the result is passed to a
> function expecting a plain HOST_WIDE_INT (ie, gimple_expand_builtin_powi).
>
> I sanity checked the patch on x86_64-linux and OP reported that on AVR the
> patch fixes the regression.
>
> Is it ok?
Ok. It looks like we might want to elimiate abs_hwi alltogether
in favor of absu_hwi?
Thanks,
Richard.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Patch] Fix tree-optimization/49963
2011-08-18 10:10 ` Richard Guenther
@ 2011-08-18 12:09 ` Paolo Carlini
2011-08-18 12:37 ` Richard Guenther
0 siblings, 1 reply; 5+ messages in thread
From: Paolo Carlini @ 2011-08-18 12:09 UTC (permalink / raw)
To: Richard Guenther; +Cc: gcc-patches, Sebastian Pop
On 08/18/2011 10:30 AM, Richard Guenther wrote:
> Ok.
Great. Thus I'm going ahead and committing the patch to fix the regression.
> It looks like we might want to elimiate abs_hwi alltogether in favor
> of absu_hwi?
Indeed, you are totally right. But I'd rather work on that as a followup
patch (maybe with Sebastian' help?) because it doesn't seem completely
trivial: I'm mostly worried by the existing uses of abs_hwi *inside*
hwint.c itself not but those two remaining instances outside of it
(which would be possible to replace immediately).
Paolo.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Patch] Fix tree-optimization/49963
2011-08-18 12:09 ` Paolo Carlini
@ 2011-08-18 12:37 ` Richard Guenther
0 siblings, 0 replies; 5+ messages in thread
From: Richard Guenther @ 2011-08-18 12:37 UTC (permalink / raw)
To: Paolo Carlini; +Cc: gcc-patches, Sebastian Pop
On Thu, 18 Aug 2011, Paolo Carlini wrote:
> On 08/18/2011 10:30 AM, Richard Guenther wrote:
> > Ok.
> Great. Thus I'm going ahead and committing the patch to fix the regression.
> > It looks like we might want to elimiate abs_hwi alltogether in favor of
> > absu_hwi?
> Indeed, you are totally right. But I'd rather work on that as a followup patch
> (maybe with Sebastian' help?) because it doesn't seem completely trivial: I'm
> mostly worried by the existing uses of abs_hwi *inside* hwint.c itself not but
> those two remaining instances outside of it (which would be possible to
> replace immediately).
Sure as a followup.
Richard.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2011-08-18 11:05 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-08-17 20:05 [Patch] Fix tree-optimization/49963 Paolo Carlini
2011-08-17 21:52 ` Georg-Johann Lay
2011-08-18 10:10 ` Richard Guenther
2011-08-18 12:09 ` Paolo Carlini
2011-08-18 12:37 ` Richard Guenther
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).