diff mbox

pr43550 - remove unnecessary uxts in bswap

Message ID 54093647.3000200@linaro.org
State New
Headers show

Commit Message

Kugan Vivekanandarajah Sept. 5, 2014, 4:04 a.m. UTC 
Hi All,

For the bswap built-in, there are unnecessary uxts generated as reported
in pr43550. Can we rely on the argument being unsigned and set the
SUBREG promoted accordingly.

At least in ARM ABI, arguments are supposed to be properly zero/sign
extended.

Any thoughts?

Bootstrapped and regression tested on x86_64 and arm.

Thanks,
Kugan

gcc/testsuite
2014-09-05  Kugan Vivekanandarajah  <kuganv@linaro.org>

	PR target/43550
	* gcc.target/arm/pr43550.c: New test.

gcc/
2014-09-05  Kugan Vivekanandarajah  <kuganv@linaro.org>

	PR target/43550
	* builtins.c (expand_builtin_bswap): Generate promoted subreg.

Comments

Jeff Law Sept. 11, 2014, 9:04 p.m. UTC | #1 
On 09/04/14 22:04, Kugan wrote:
> Hi All,
>
> For the bswap built-in, there are unnecessary uxts generated as reported
> in pr43550. Can we rely on the argument being unsigned and set the
> SUBREG promoted accordingly.
>
> At least in ARM ABI, arguments are supposed to be properly zero/sign
> extended.
>
> Any thoughts?
>
> Bootstrapped and regression tested on x86_64 and arm.
>
> Thanks,
> Kugan
>
> gcc/testsuite
> 2014-09-05  Kugan Vivekanandarajah  <kuganv@linaro.org>
>
> 	PR target/43550
> 	* gcc.target/arm/pr43550.c: New test.
>
> gcc/
> 2014-09-05  Kugan Vivekanandarajah  <kuganv@linaro.org>
>
> 	PR target/43550
> 	* builtins.c (expand_builtin_bswap): Generate promoted subreg.
I'm not 100% sure you can depend on arguments being extended.  It'll 
often be true, but I don't think it's something you can rely on.

Jeff
diff mbox

Patch

diff --git a/gcc/builtins.c b/gcc/builtins.c
index e5a9b4d..a2f2358 100644
--- a/gcc/builtins.c
+++ b/gcc/builtins.c
@@ -4517,6 +4517,12 @@  expand_builtin_bswap (enum machine_mode target_mode, tree exp, rtx target,
   if (!validate_arglist (exp, INTEGER_TYPE, VOID_TYPE))
     return NULL_RTX;
 
+  if (!target
+      && (GET_MODE_PRECISION (word_mode)
+	  > TYPE_PRECISION (TREE_TYPE (exp))))
+    target = gen_lowpart_SUBREG (TYPE_MODE (TREE_TYPE (exp)),
+				   gen_reg_rtx (word_mode));
+
   arg = CALL_EXPR_ARG (exp, 0);
   op0 = expand_expr (arg,
 		     subtarget && GET_MODE (subtarget) == target_mode
@@ -4528,8 +4534,13 @@  expand_builtin_bswap (enum machine_mode target_mode, tree exp, rtx target,
   target = expand_unop (target_mode, bswap_optab, op0, target, 1);
 
   gcc_assert (target);
-
-  return convert_to_mode (target_mode, target, 1);
+  target = convert_to_mode (target_mode, target, 1);
+  if (GET_CODE (target) == SUBREG)
+    {
+      SUBREG_PROMOTED_VAR_P (target) = 1;
+      SUBREG_PROMOTED_SET (target, SRP_UNSIGNED);
+    }
+  return target;
 }
 
 /* Expand a call to a unary builtin in EXP.
diff --git a/gcc/testsuite/gcc.target/arm/pr43550.c b/gcc/testsuite/gcc.target/arm/pr43550.c
index e69de29..7e4b2e0 100644
--- a/gcc/testsuite/gcc.target/arm/pr43550.c
+++ b/gcc/testsuite/gcc.target/arm/pr43550.c
@@ -0,0 +1,21 @@ 
+/* { dg-do compile } */
+/* { dg-options "-O2" } */
+/* { dg-final { scan-assembler-times "uxt" 0 } } */
+
+typedef unsigned short uint16_t;
+typedef unsigned int uint32_t;
+
+uint16_t s16 (uint16_t v)
+{
+  return v >> 8 | v << 8;
+}
+
+uint16_t _s16 (uint16_t v)
+{
+  return __builtin_bswap16 (v);
+}
+
+uint32_t s32 (uint32_t v)
+{
+  return __builtin_bswap32 (v);
+}