This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [power7-meissner] Add __builtin_bswap{16,64} support to powerpc


On Thu, Jun 11, 2009 at 07:54:06PM -0700, Richard Henderson wrote:
> Michael Meissner wrote:
> >+(define_expand "bswaphi2"
> >+  [(use (match_operand:SI 0 "gpc_reg_operand" ""))
> >+   (use (match_operand:SI 1 "gpc_reg_operand" ""))]
> >+  "TARGET_POWERPC"
> >+  {
> >+    rtx inp = gen_reg_rtx (HImode);
> >+    rtx out = gen_reg_rtx (HImode);
> >+
> >+    emit_move_insn (inp, simplify_gen_subreg (HImode, operands[1], 
> >SImode, 2));
> >+    emit_insn (gen_bswaphi2_internal (out, inp));
> >+    emit_insn (gen_zero_extendhisi2 (operands[0], out));
> >+    DONE;
> >+  })
> 
> Please don't put all this SImode nonsense in a pattern
> that, by name and middle-end convention, is supposed to
> operate on HImode values.

Yep.  I used a function I used to build function types for for the altivec/vsx
builtins to cache common copies and it internally converted HImode to SImode
for integer constants.  So, I just avoided using that function and called
build_function_type_list directly.
 
> As for the non-existance of bswap16 as a generic builtin...
> frankly I don't see why we can't add one for the sake of
> consistency.

Yes, I think we probably should add bswap16.  Thinking about vector types, it
may be useful to think about bswap128 and bswap256 as well.

Here is the patch I applied.

2009-06-12  Michael Meissner  <meissner@linux.vnet.ibm.com>

	* config/rs6000/rs6000-c.c (rs6000_cpu_cpp_builtins): Define
	__HAVE_BSWAP__ to let people know they can use the bswap
	builtins.

	* config/rs6000/rs6000.c (rs6000_init_builtins): Make the bswap16
	builtin take and return unsigned short ints.

	* config/rs6000/rs6000.md (bswaphi2*): Use HImode as argument and
	return type.

Index: gcc/config/rs6000/rs6000-c.c
===================================================================
--- gcc/config/rs6000/rs6000-c.c	(revision 148152)
+++ gcc/config/rs6000/rs6000-c.c	(working copy)
@@ -364,6 +364,9 @@ rs6000_cpu_cpp_builtins (cpp_reader *pfi
       builtin_define ("__builtin_vsx_xvnmsubmsp=__builtin_vsx_xvnmsubsp");
     }
 
+  /* Tell users they can use __builtin_bswap{16,64}.  */
+  builtin_define ("__HAVE_BSWAP__");
+
   /* May be overridden by target configuration.  */
   RS6000_CPU_CPP_ENDIAN_BUILTINS();
 
Index: gcc/config/rs6000/rs6000.c
===================================================================
--- gcc/config/rs6000/rs6000.c	(revision 148399)
+++ gcc/config/rs6000/rs6000.c	(working copy)
@@ -10850,9 +10850,10 @@ rs6000_init_builtins (void)
     }
   if (TARGET_POWERPC)
     {
-      tree ftype = builtin_function_type (SImode, SImode, VOIDmode, VOIDmode,
-					  RS6000_BUILTIN_BSWAP_HI,
-					  "__builtin_bswap16");
+      /* Don't use builtin_function_type here, as it maps HI/QI to SI.  */
+      tree ftype = build_function_type_list (unsigned_intHI_type_node,
+					     unsigned_intHI_type_node,
+					     NULL_TREE);
       def_builtin (MASK_POWERPC, "__builtin_bswap16", ftype,
 		   RS6000_BUILTIN_BSWAP_HI);
     }
Index: gcc/config/rs6000/rs6000.md
===================================================================
--- gcc/config/rs6000/rs6000.md	(revision 148400)
+++ gcc/config/rs6000/rs6000.md	(working copy)
@@ -2266,43 +2266,37 @@ (define_expand "parity<mode>2"
     DONE;
   })
 
-;; For the generation phase, restrict ourselves to registers, and let the
-;; optimization passes remove the extra moves.  Take SI input and provide SI
-;; output, and internally convert to HI.
-(define_expand "bswaphi2"
-  [(use (match_operand:SI 0 "gpc_reg_operand" ""))
-   (use (match_operand:SI 1 "gpc_reg_operand" ""))]
-  "TARGET_POWERPC"
-  {
-    rtx inp = gen_reg_rtx (HImode);
-    rtx out = gen_reg_rtx (HImode);
-
-    emit_move_insn (inp, simplify_gen_subreg (HImode, operands[1], SImode, 2));
-    emit_insn (gen_bswaphi2_internal (out, inp));
-    emit_insn (gen_zero_extendhisi2 (operands[0], out));
-    DONE;
-  })
-
 ;; Since the hardware zeros the upper part of the register, save generating the
-;; AND immediate if converting to unsigned
-(define_insn "*bswaphi2_extendsi"
-  [(set (match_operand:SI 0 "gpc_reg_operand" "=r")
-	(zero_extend:SI
+;; AND immediate if we are converting to unsigned
+(define_insn "*bswaphi2_extenddi"
+  [(set (match_operand:DI 0 "gpc_reg_operand" "=r")
+	(zero_extend:DI
 	 (bswap:HI (match_operand:HI 1 "memory_operand" "Z"))))]
-  "TARGET_POWERPC"
+  "TARGET_POWERPC64"
   "lhbrx %0,%y1"
   [(set_attr "length" "4")
    (set_attr "type" "load")])
 
-(define_insn "*bswaphi2_extenddi"
-  [(set (match_operand:DI 0 "gpc_reg_operand" "=r")
-	(zero_extend:DI
+(define_insn "*bswaphi2_extendsi"
+  [(set (match_operand:SI 0 "gpc_reg_operand" "=r")
+	(zero_extend:SI
 	 (bswap:HI (match_operand:HI 1 "memory_operand" "Z"))))]
-  "TARGET_POWERPC64"
+  "TARGET_POWERPC"
   "lhbrx %0,%y1"
   [(set_attr "length" "4")
    (set_attr "type" "load")])
 
+(define_expand "bswaphi2"
+  [(parallel [(set (match_operand:HI 0 "reg_or_mem_operand" "")
+		   (bswap:HI
+		    (match_operand:HI 1 "reg_or_mem_operand" "")))
+	      (clobber (match_scratch:SI 2 ""))])]
+  ""
+{
+  if (!REG_P (operands[0]) && !REG_P (operands[1]))
+    operands[1] = force_reg (SImode, operands[1]);
+})
+
 (define_insn "bswaphi2_internal"
   [(set (match_operand:HI 0 "reg_or_mem_operand" "=r,Z,&r")
 	(bswap:HI


-- 
Michael Meissner, IBM
4 Technology Place Drive, MS 2203A, Westford, MA, 01886, USA
meissner@linux.vnet.ibm.com


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]