On 06/08/14 23:29, Richard Biener wrote:
> On Wed, Aug 6, 2014 at 3:21 PM, Kugan <kugan.vivekanandara...@linaro.org>
> wrote:
>> On 06/08/14 22:09, Richard Biener wrote:
>>> On Tue, Aug 5, 2014 at 4:21 PM, Jakub Jelinek <ja...@redhat.com> wrote:
>>>> On Tue, Aug 05, 2014 at 04:17:41PM +0200, Richard Biener wrote:
>>>>> what's the semantic of setting SRP_SIGNED_AND_UNSIGNED
>>>>> on the subreg? That is, for the created (subreg:lhs_mode
>>>>> (reg:<PROMOTE_MODE of ssa> N))?
>>>>
>>>> SRP_SIGNED_AND_UNSIGNED on a subreg should mean that
>>>> the subreg is both zero and sign extended, which means
>>>> that the topmost bit of the narrower mode is known to be zero,
>>>> and all bits above it in the wider mode are known to be zero too.
>>>> SRP_SIGNED means that the topmost bit of the narrower mode is
>>>> either 0 or 1 and depending on that the above wider mode bits
>>>> are either all 0 or all 1.
>>>> SRP_UNSIGNED means that regardless of the topmost bit value,
>>>> all above wider mode bits are 0.
>>>
>>> Ok, then from the context of the patch we already know that
>>> either SRP_UNSIGNED or SRP_SIGNED is true which means
>>> that the value is sign- or zero-extended.
>>>
>>> I suppose inside promoted_for_type_p
>>> TYPE_MODE (TREE_TYPE (ssa)) == lhs_mode, I'm not sure
>>> why you pass !unsignedp as lhs_uns.
>>
>> In expand_expr_real_1, it is already known that it is promoted for
>> unsigned_p and we are setting SUBREG_PROMOTED_SET (temp, unsignedp).
>>
>> If we can prove that it is also promoted for !unsignedp, we can set
>> SUBREG_PROMOTED_SET (temp, SRP_SIGNED_AND_UNSIGNED).
>>
>> promoted_for_type_p should prove this based on the value range info.
>>
>>>
>>> Now, from 'ssa' alone we can't tell anything about a larger mode
>>> registers value if that is either zero- or sign-extended. But we
>>> know that those bits are properly zero-extended if unsignedp
>>> and properly sign-extended if !unsignedp?
>>>
>>> So what the predicate tries to prove is that sign- and zero-extending
>>> results in the same larger-mode value. This is true if the
>>> MSB of the smaller mode is not set.
>>>
>>> Let's assume that smaller mode is that of 'ssa' then the test
>>> is just
>>>
>>> return (!tree_int_cst_sign_bit (min) && !tree_int_cst_sign_bit (max));
>>>
>>> no?
>>
>> hmm, is this because we will never have a call to promoted_for_type_p
>> with same sign (ignoring PROMOTE_MODE) for 'ssa' and the larger mode.
>> The case with larger mode signed and 'ssa' unsigned will not work.
>> Therefore larger mode unsigned and 'ssa' signed will be the only case
>> that we should consider.
>>
>> However, with PROMOTE_MODE, isnt that we will miss some cases with this.
>
> No, PROMOTE_MODE will still either sign- or zero-extend. If either
> results in zeros in the upper bits then PROMOTE_MODE doesn't matter.
>
Thanks for the explanation. Please find the attached patch that
implements this. I have updated the comments and predicate to match this.
Bootstrap tested on x86_64-unknown-linux-gnu and regression tested on
x86_64-unknown-linux-gnu and arm-none-linux-gnueabi with no new
regressions. Is this OK?
Thanks,
Kugan
gcc/
2014-08-07 Kugan Vivekanandarajah <kug...@linaro.org>
* calls.c (precompute_arguments): Check
promoted_for_signed_and_unsigned_p and set the promoted mode.
(promoted_for_signed_and_unsigned_p): New function.
(expand_expr_real_1): Check promoted_for_signed_and_unsigned_p
and set the promoted mode.
* expr.h (promoted_for_signed_and_unsigned_p): New function definition.
* cfgexpand.c (expand_gimple_stmt_1): Call emit_move_insn if
SUBREG is promoted with SRP_SIGNED_AND_UNSIGNED.
gcc/testsuite
2014-08-07 Kugan Vivekanandarajah <kug...@linaro.org>
* gcc.dg/zero_sign_ext_test.c: New test.
diff --git a/gcc/calls.c b/gcc/calls.c
index 00c5028..4285ec1 100644
--- a/gcc/calls.c
+++ b/gcc/calls.c
@@ -1484,7 +1484,10 @@ precompute_arguments (int num_actuals, struct arg_data
*args)
args[i].initial_value
= gen_lowpart_SUBREG (mode, args[i].value);
SUBREG_PROMOTED_VAR_P (args[i].initial_value) = 1;
- SUBREG_PROMOTED_SET (args[i].initial_value, args[i].unsignedp);
+ if (promoted_for_signed_and_unsigned_p (args[i].tree_value, mode))
+ SUBREG_PROMOTED_SET (args[i].initial_value,
SRP_SIGNED_AND_UNSIGNED);
+ else
+ SUBREG_PROMOTED_SET (args[i].initial_value, args[i].unsignedp);
}
}
}
diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c
index f98c322..b14626c 100644
--- a/gcc/cfgexpand.c
+++ b/gcc/cfgexpand.c
@@ -3309,7 +3309,13 @@ expand_gimple_stmt_1 (gimple stmt)
GET_MODE (target), temp, unsignedp);
}
- convert_move (SUBREG_REG (target), temp, unsignedp);
+ if ((SUBREG_PROMOTED_GET (target) == SRP_SIGNED_AND_UNSIGNED)
+ && (GET_CODE (temp) == SUBREG)
+ && (GET_MODE (target) == GET_MODE (temp))
+ && (GET_MODE (SUBREG_REG (target)) == GET_MODE (SUBREG_REG
(temp))))
+ emit_move_insn (SUBREG_REG (target), SUBREG_REG (temp));
+ else
+ convert_move (SUBREG_REG (target), temp, unsignedp);
}
else if (nontemporal && emit_storent_insn (target, temp))
;
diff --git a/gcc/expr.c b/gcc/expr.c
index 1242031..c217b9a 100644
--- a/gcc/expr.c
+++ b/gcc/expr.c
@@ -68,6 +68,7 @@ along with GCC; see the file COPYING3. If not see
#include "tree-ssa-address.h"
#include "cfgexpand.h"
#include "builtins.h"
+#include "tree-ssa.h"
#ifndef STACK_PUSH_CODE
#ifdef STACK_GROWS_DOWNWARD
@@ -9224,6 +9225,35 @@ expand_expr_real_2 (sepops ops, rtx target, enum
machine_mode tmode,
}
#undef REDUCE_BIT_FIELD
+/* Return TRUE if value in SSA is zero and sign extended for wider mode MODE
+ using value range information stored. Return FALSE otherwise.
+
+ This is used to check if SUBREG is zero and sign extended and to set
+ promoted mode SRP_SIGNED_AND_UNSIGNED to SUBREG. */
+
+bool
+promoted_for_signed_and_unsigned_p (tree ssa, enum machine_mode mode)
+{
+ wide_int min, max;
+
+ if (ssa == NULL_TREE
+ || TREE_CODE (ssa) != SSA_NAME
+ || !INTEGRAL_TYPE_P (TREE_TYPE (ssa))
+ || (TYPE_PRECISION (TREE_TYPE (ssa)) > GET_MODE_PRECISION (mode)))
+ return false;
+
+ /* Return FALSE if value_range is not recorded for SSA. */
+ if (get_range_info (ssa, &min, &max) != VR_RANGE)
+ return false;
+
+ /* Return true (to set SRP_SIGNED_AND_UNSIGNED to SUBREG) if MSB of the
smaller
+ mode is not set (i.e. MSB of ssa is not set). */
+ if (!wi::neg_p (min, SIGNED) && !wi::neg_p(max, SIGNED))
+ return true;
+ else
+ return false;
+
+}
/* Return TRUE if expression STMT is suitable for replacement.
Never consider memory loads as replaceable, because those don't ever lead
@@ -9527,7 +9557,10 @@ expand_expr_real_1 (tree exp, rtx target, enum
machine_mode tmode,
temp = gen_lowpart_SUBREG (mode, decl_rtl);
SUBREG_PROMOTED_VAR_P (temp) = 1;
- SUBREG_PROMOTED_SET (temp, unsignedp);
+ if (promoted_for_signed_and_unsigned_p (ssa_name, mode))
+ SUBREG_PROMOTED_SET (temp, SRP_SIGNED_AND_UNSIGNED);
+ else
+ SUBREG_PROMOTED_SET (temp, unsignedp);
return temp;
}
diff --git a/gcc/expr.h b/gcc/expr.h
index 6a1d3ab..a429509 100644
--- a/gcc/expr.h
+++ b/gcc/expr.h
@@ -440,6 +440,7 @@ extern rtx expand_expr_real_1 (tree, rtx, enum machine_mode,
enum expand_modifier, rtx *, bool);
extern rtx expand_expr_real_2 (sepops, rtx, enum machine_mode,
enum expand_modifier);
+extern bool promoted_for_signed_and_unsigned_p (tree, enum machine_mode);
/* Generate code for computing expression EXP.
An rtx for the computed value is returned. The value is never null.
diff --git a/gcc/testsuite/gcc.dg/zero_sign_ext_test.c
b/gcc/testsuite/gcc.dg/zero_sign_ext_test.c
index e69de29..6a52678 100644
--- a/gcc/testsuite/gcc.dg/zero_sign_ext_test.c
+++ b/gcc/testsuite/gcc.dg/zero_sign_ext_test.c
@@ -0,0 +1,136 @@
+extern void abort (void);
+
+/* { dg-options "-O2" } */
+/* { dg-do run } */
+
+#define TYPE_MAX(type, sign) \
+ ((!sign) ? ((1 << (sizeof (type) * 8 - 1)) - 1) : \
+ ((1 << (sizeof (type) * 8)) - 1))
+#define TYPE_MIN(type, sign) \
+ ((!sign) ? -(1 << (sizeof (type) * 8 - 1)) : 0)
+
+#define TEST_FN(NAME, ARG_TYPE, RET_TYPE, CAST_TYPE, VAL, VR_MIN, VR_MAX)\
+ __attribute__((noinline, noclone)) RET_TYPE \
+ NAME (ARG_TYPE arg){ \
+ RET_TYPE ret = VAL; \
+ if (arg + 1 < VR_MIN || arg + 1 > VR_MAX) return ret; \
+ /* Value Range of arg at this point will be [VR_min, VR_max]. */\
+ arg = arg + VAL; \
+ ret = (CAST_TYPE)arg; \
+ return arg; \
+ }
+
+/* Signed to signed conversion with value in-range. */
+TEST_FN (foo1, short, short, char, 1, TYPE_MIN (char, 0), TYPE_MAX (char, 0));
+TEST_FN (foo2, short, short, char, 1, TYPE_MIN (char, 0) + 1,\
+ TYPE_MAX (char, 0) - 1);
+
+/* Signed to signed conversion with value not in-range. */
+TEST_FN (foo3, short, short, char, -1, TYPE_MIN (short, 0) + 1, 100);
+TEST_FN (foo4, short, short, char, 1, 12, TYPE_MAX (short, 0) + 1);
+
+/* Unsigned to unsigned conversion with value in-range. */
+TEST_FN (foo5, unsigned short, unsigned short, unsigned char, 1,\
+ TYPE_MIN (char, 1) + 1, TYPE_MAX (char, 1) - 1);
+TEST_FN (foo6, unsigned short, unsigned short, unsigned char, 1,\
+ TYPE_MIN (char, 1), TYPE_MAX (char, 1));
+
+/* Unsigned to unsigned conversion with value not in-range. */
+TEST_FN (foo7, unsigned short, unsigned short, unsigned char, 1,\
+ TYPE_MIN (short, 1) + 1, TYPE_MAX (short, 1) - 1);
+TEST_FN (foo8, unsigned short, unsigned short, unsigned char, 1,\
+ TYPE_MIN (short, 1), TYPE_MAX (short, 1));
+
+/* Signed to unsigned conversion with value range positive. */
+TEST_FN (foo9, short, short, unsigned char, -1, 1,\
+ TYPE_MAX (char, 1) - 1);
+TEST_FN (foo10, short, short, unsigned char, 1, 0,\
+ TYPE_MAX (char, 1));
+
+/* Signed to unsigned conversion with value range negative. */
+TEST_FN (foo11, short, short, unsigned char, 1,\
+ TYPE_MIN (char, 0) + 1, TYPE_MAX (char, 0) - 1);
+TEST_FN (foo12, short, short, unsigned char, 1,\
+ TYPE_MIN (char, 0), TYPE_MAX (char, 0));
+
+/* Unsigned to Signed conversion with value range in signed equiv range. */
+TEST_FN (foo13, unsigned short, unsigned short, char, 1,\
+ TYPE_MIN (char, 1) + 1, TYPE_MAX (char, 0) - 1);
+TEST_FN (foo14, unsigned short, unsigned short, char, 1,\
+ TYPE_MIN (char, 1), TYPE_MAX (char, 0));
+
+/* Unsigned to Signed conversion with value range not-in signed range. */
+TEST_FN (foo15, unsigned short, unsigned short, char, 1,\
+ TYPE_MIN (char, 1) + 1, TYPE_MAX (char, 1) - 1);
+TEST_FN (foo16, unsigned short, unsigned short, char, 1,\
+ TYPE_MIN (char, 1), TYPE_MAX (char, 1));
+
+int main ()
+{
+ /* Signed to signed conversion with value in-range. */
+ /* arg + 1. */
+ if (foo1 (-32) != -31)
+ abort ();
+ /* arg + 1. */
+ if (foo2 (32) != 33)
+ abort ();
+
+ /* Signed to signed conversion with value not in-range. */
+ /* arg - 1. */
+ if (foo3 (-512) != -513)
+ abort ();
+ /* arg + 1. */
+ if (foo4 (512) != 513)
+ abort ();
+
+ /* Unsigned to unsigned conversion with value in-range. */
+ /* arg + 1. */
+ if (foo5 (64) != 65)
+ abort ();
+ /* arg + 1. */
+ if (foo6 (64) != 65)
+ abort ();
+
+ /* Unsigned to unsigned conversion with value not in-range. */
+ /* arg + 1. */
+ if (foo7 (512) != 513)
+ abort ();
+ /* arg + 1. */
+ if (foo8 (512) != 513)
+ abort ();
+
+ /* Signed to unsigned conversion with value range positive. */
+ /* arg - 1. */
+ if (foo9 (2) != 1)
+ abort ();
+ /* arg + 1. */
+ if (foo10 (2) != 3)
+ abort ();
+
+ /* Signed to unsigned conversion with value range negative. */
+ /* arg + 1. */
+ if (foo11 (-125) != -124)
+ abort ();
+ /* arg + 1. */
+ if (foo12 (-125) != -124)
+ abort ();
+
+ /* Unsigned to Signed conversion with value range in signed equiv range. */
+ /* arg + 1. */
+ if (foo13 (125) != 126)
+ abort ();
+ /* arg + 1. */
+ if (foo14 (125) != 126)
+ abort ();
+
+ /* Unsigned to Signed conversion with value range not-in signed range. */
+ /* arg + 1. */
+ if (foo15 (250) != 251)
+ abort ();
+ /* arg + 1. */
+ if (foo16 (250) != 251)
+ abort ();
+
+ return 0;
+}
+
