diff mbox

[vect] Ask for review and approving the patch about vect and loongson

Message ID 87bpafjsxq.fsf@firetop.home
State New
Headers show

Commit Message

Richard Sandiford July 10, 2010, 2:46 p.m. UTC 
The updated vect.patch is OK.

The target-supports.patch is OK if you break the line immediately before
the "&&"; the lines are too long as-is.

As far as loongson.patch goes:


Instead rename loongson_pmull<V_suffix> to mul<mode>3 and add #defines
to mips.c to make CODE_FOR_loongson_pmullh an alias for CODE_FOR_mulhi3,
etc.  Grep for CODE_FOR_loongson to see where I mean.

The patch adds patterns for shifts right but not shifts left.  Was that
deliberate?

The same rename-and-#define common would apply to the shifts, but I'm
worried.  The loongson insns are all "infinite precision" shifts
(any V2HI << 16 == 0), whereas MIPS is a SHIFT_COUNT_TRUNCATED
target (implying V2HI << 16 should be an identity operation).
This could in theory cause us to miscompile things like:

   v2hi >> (shift & 15)

and although I can't come up with a testcase, I think the problem
is still there.

Admittedly this means the current code is wrong too.  It should be
using UNSPECs instead of shift rtxes.

One fix would be to make SHIFT_COUNT_TRUNCATED take a mode argument
(and turn it into a target hook at the same time).

Richard
diff mbox

Patch

Index: config/mips/loongson.md
===================================================================
--- config/mips/loongson.md	(revision 161865)
+++ config/mips/loongson.md	(working copy)
@@ -352,6 +352,16 @@ 
   "pmulh<V_suffix>\t%0,%1,%2"
   [(set_attr "type" "fmul")])
 
+;; Standard pattern mulm3
+(define_expand "mul<mode>3"
+  [(set (match_operand:VH 0 "register_operand" "=f")
+        (unspec:VH [(match_operand:VH 1 "register_operand" "f")
+                    (match_operand:VH 2 "register_operand" "f")]
+                   UNSPEC_LOONGSON_PMULL))]
+  "TARGET_HARD_FLOAT && TARGET_LOONGSON_VECTORS"
+  "")
+
+
 ;; Multiply signed integers and store low result.
 (define_insn "loongson_pmull<V_suffix>"
   [(set (match_operand:VH 0 "register_operand" "=f")