diff mbox

[expand] Fix for PR rtl-optimization/79121 incorrect expansion of extend plus left shift

Message ID 70b62c30-956f-0c3c-92f1-0cd5f1acdb2d@arm.com
State New
Headers show

Commit Message

Richard Earnshaw (lists) Jan. 18, 2017, 6:08 p.m. UTC 
PR 79121 is a silent wrong code regression where, when generating a
shift from an extended value moving from one to two machine registers,
the type of the right shift is for the most significant word should be
determined by the signedness of the inner type, not the signedness of
the result type.

gcc:
	PR rtl-optimization/79121
	* expr.c (expand_expr_real_2, case LSHIFT_EXPR): Look at the signedness
	of the inner type when shifting an extended value.

testsuite:
	* gcc.c-torture/execute/pr79121.c: New test.

Bootstrapped on x86_64 and cross-tested on ARM.

OK for trunk and GCC-6?

R.

Comments

Jeff Law Jan. 18, 2017, 9:07 p.m. UTC | #1 
On 01/18/2017 11:08 AM, Richard Earnshaw (lists) wrote:
> PR 79121 is a silent wrong code regression where, when generating a
> shift from an extended value moving from one to two machine registers,
> the type of the right shift is for the most significant word should be
> determined by the signedness of the inner type, not the signedness of
> the result type.
>
> gcc:
> 	PR rtl-optimization/79121
> 	* expr.c (expand_expr_real_2, case LSHIFT_EXPR): Look at the signedness
> 	of the inner type when shifting an extended value.
>
> testsuite:
> 	* gcc.c-torture/execute/pr79121.c: New test.
>
> Bootstrapped on x86_64 and cross-tested on ARM.
I had to refamiliarize myself with this code and nearly got the analysis 
wrong (again).

Due to the copying of the low word into the high word we have to select 
the type of shift based on the type of the object that was the source of 
the NOP conversion.  The code currently makes that determination based 
on the signedness of the shift, which is wrong.


OK for the trunk.

jeff
diff mbox

Patch

diff --git a/gcc/expr.c b/gcc/expr.c
index 4c54faf..cae13a4 100644
--- a/gcc/expr.c
+++ b/gcc/expr.c
@@ -9093,7 +9093,6 @@  expand_expr_real_2 (sepops ops, rtx target, machine_mode tmode,
 	if (code == LSHIFT_EXPR
 	    && target
 	    && REG_P (target)
-	    && ! unsignedp
 	    && mode == GET_MODE_WIDER_MODE (word_mode)
 	    && GET_MODE_SIZE (mode) == 2 * GET_MODE_SIZE (word_mode)
 	    && TREE_CONSTANT (treeop1)
@@ -9114,6 +9113,8 @@  expand_expr_real_2 (sepops ops, rtx target, machine_mode tmode,
 		    rtx_insn *seq, *seq_old;
 		    unsigned int high_off = subreg_highpart_offset (word_mode,
 								    mode);
+		    bool extend_unsigned
+		      = TYPE_UNSIGNED (TREE_TYPE (gimple_assign_rhs1 (def)));
 		    rtx low = lowpart_subreg (word_mode, op0, mode);
 		    rtx dest_low = lowpart_subreg (word_mode, target, mode);
 		    rtx dest_high = simplify_gen_subreg (word_mode, target,
@@ -9125,7 +9126,8 @@  expand_expr_real_2 (sepops ops, rtx target, machine_mode tmode,
 		    start_sequence ();
 		    /* dest_high = src_low >> (word_size - C).  */
 		    temp = expand_variable_shift (RSHIFT_EXPR, word_mode, low,
-						  rshift, dest_high, unsignedp);
+						  rshift, dest_high,
+						  extend_unsigned);
 		    if (temp != dest_high)
 		      emit_move_insn (dest_high, temp);
 
diff --git a/gcc/testsuite/gcc.c-torture/execute/pr79121.c b/gcc/testsuite/gcc.c-torture/execute/pr79121.c
new file mode 100644
index 0000000..9fca7fb
--- /dev/null
+++ b/gcc/testsuite/gcc.c-torture/execute/pr79121.c
@@ -0,0 +1,34 @@ 
+extern void abort (void);
+
+__attribute__ ((noinline, noclone)) unsigned long long f1 (int x)
+{
+  return ((unsigned long long) x) << 4;
+}
+
+__attribute__ ((noinline, noclone)) long long f2 (unsigned x)
+{
+  return ((long long) x) << 4;
+}
+
+__attribute__ ((noinline, noclone)) unsigned long long f3 (unsigned x)
+{
+  return ((unsigned long long) x) << 4;
+}
+
+__attribute__ ((noinline, noclone)) long long f4 (int x)
+{
+  return ((long long) x) << 4;
+}
+
+int main ()
+{
+  if (f1 (0xf0000000) != 0xffffffff00000000)
+    abort ();
+  if (f2 (0xf0000000) != 0xf00000000)
+    abort ();
+  if (f3 (0xf0000000) != 0xf00000000)
+    abort ();
+  if (f4 (0xf0000000) != 0xffffffff00000000)
+    abort ();
+  return 0;
+}