From patchwork Mon Jul 12 09:49:40 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Subject: [vect] Ask for review and approving the patch about vect and loongson Date: Sun, 11 Jul 2010 23:49:40 -0000 From: Eric Fisher X-Patchwork-Id: 58588 Message-Id: To: Richard Guenther , gcc-patches , rdsandiford@googlemail.com 2010/7/10 Richard Sandiford : > 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. OK, I've updated the patch in the attachment. > As far as loongson.patch goes: > > Index: config/mips/loongson.md > =================================================================== > --- config/mips/loongson.md     (revision 161865) > +++ config/mips/loongson.md     (working copy) > @@ -352,6 +352,16 @@ >   "pmulh\t%0,%1,%2" >   [(set_attr "type" "fmul")]) > > +;; Standard pattern mulm3 > +(define_expand "mul3" > +  [(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" >   [(set (match_operand:VH 0 "register_operand" "=f") > > Instead rename loongson_pmull to mul3 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. Thanks for the helpful information. I've changed the patch in the attachment. > The patch adds patterns for shifts right but not shifts left.  Was that > deliberate? Oh, I missed it. Now I've added it. > 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 > I'm going to take a look at this problem. Do you think that it is all right to first submit the current patch and leave this problem in the next one separately? Thanks Eric Index: config/mips/loongson.md =================================================================== --- config/mips/loongson.md (revision 162065) +++ config/mips/loongson.md (working copy) @@ -353,7 +353,7 @@ [(set_attr "type" "fmul")]) ;; Multiply signed integers and store low result. -(define_insn "loongson_pmull" +(define_insn "mul3" [(set (match_operand:VH 0 "register_operand" "=f") (unspec:VH [(match_operand:VH 1 "register_operand" "f") (match_operand:VH 2 "register_operand" "f")] @@ -413,7 +413,7 @@ [(set_attr "type" "fmul")]) ;; Shift left logical. -(define_insn "loongson_psll" +(define_insn "ashl3" [(set (match_operand:VWH 0 "register_operand" "=f") (ashift:VWH (match_operand:VWH 1 "register_operand" "f") (match_operand:SI 2 "register_operand" "f")))] @@ -422,7 +422,7 @@ [(set_attr "type" "fmul")]) ;; Shift right arithmetic. -(define_insn "loongson_psra" +(define_insn "ashr3" [(set (match_operand:VWH 0 "register_operand" "=f") (ashiftrt:VWH (match_operand:VWH 1 "register_operand" "f") (match_operand:SI 2 "register_operand" "f")))] @@ -431,7 +431,7 @@ [(set_attr "type" "fdiv")]) ;; Shift right logical. -(define_insn "loongson_psrl" +(define_insn "lshr3" [(set (match_operand:VWH 0 "register_operand" "=f") (lshiftrt:VWH (match_operand:VWH 1 "register_operand" "f") (match_operand:SI 2 "register_operand" "f")))] Index: config/mips/mips.c =================================================================== --- config/mips/mips.c (revision 162065) +++ config/mips/mips.c (working copy) @@ -12667,6 +12667,13 @@ AVAIL_NON_MIPS16 (cache, TARGET_CACHE_BU #define CODE_FOR_loongson_pminub CODE_FOR_uminv8qi3 #define CODE_FOR_loongson_pmulhuh CODE_FOR_umulv4hi3_highpart #define CODE_FOR_loongson_pmulhh CODE_FOR_smulv4hi3_highpart +#define CODE_FOR_loongson_pmullh CODE_FOR_mulv4hi3 +#define CODE_FOR_loongson_psllh CODE_FOR_ashlv4hi3 +#define CODE_FOR_loongson_psllw CODE_FOR_ashlv2si3 +#define CODE_FOR_loongson_psrlh CODE_FOR_lshrv4hi3 +#define CODE_FOR_loongson_psrlw CODE_FOR_lshrv2si3 +#define CODE_FOR_loongson_psrah CODE_FOR_ashrv4hi3 +#define CODE_FOR_loongson_psraw CODE_FOR_ashrv2si3 #define CODE_FOR_loongson_psubw CODE_FOR_subv2si3 #define CODE_FOR_loongson_psubh CODE_FOR_subv4hi3 #define CODE_FOR_loongson_psubb CODE_FOR_subv8qi3