On Tue, Jan 09, 2018 at 06:28:09PM +0000, Michael Collison wrote: > Patch updated per Richard's comments. Ok for trunk?
This patch adds a lot of code, much of which looks like it ought to be possible to common up using the iterators. I'm going to OK it as is, as I'd like to see this make GCC 8, and we've sat on it for long enough, but I would really appreciate futurec refactoring in this area. I'm worried about maintainability as it stands. OK. Thanks, James > > -----Original Message----- > From: Richard Sandiford [mailto:richard.sandif...@linaro.org] > Sent: Thursday, January 4, 2018 8:02 AM > To: Michael Collison <michael.colli...@arm.com> > Cc: GCC Patches <gcc-patches@gcc.gnu.org>; nd <n...@arm.com> > Subject: Re: [PATCH 5/5][AArch64] fp16fml support > > Hi Michael, > > Not a review of the full patch, just a comment about the patterns: > > Michael Collison <michael.colli...@arm.com> writes: > > +(define_expand "aarch64_fml<f16mac1>l_lane_lowv2sf" > > + [(set (match_operand:V2SF 0 "register_operand" "") > > + (unspec:V2SF [(match_operand:V2SF 1 "register_operand" "") > > + (match_operand:V4HF 2 "register_operand" "") > > + (match_operand:V4HF 3 "register_operand" "") > > + (match_operand:SI 4 "aarch64_imm2" "")] > > + VFMLA16_LOW))] > > + "TARGET_F16FML" > > +{ > > + rtx p1 = aarch64_simd_vect_par_cnst_half (V4HFmode, > > + GET_MODE_NUNITS (V4HFmode), > > + false); > > + rtx lane = GEN_INT (ENDIAN_LANE_N (GET_MODE_NUNITS (SImode), INTVAL > > (operands[4]))); > > Please use the newly-introduced aarch64_endian_lane_rtx for this. > > GET_MODE_NUNITS (SImode) doesn't seem right though, since that's always 1. > Should it be using V4HFmode instead? > > Same for the other patterns. > > Thanks, > Richard
