On Sun, Oct 01, 2017 at 02:07:57AM +0100, Michael Collison wrote: > Sorry. Here is the patch.
I think this needs a small amount fo rework in iterators.md - the names you've used don't follow conventions in that file (e.g. "V" normally has something to do with vectors) so could do with patching up. > -(define_insn "<optab>_trunc<GPF_F16:mode><GPI:mode>2" > +(define_insn "<optab>_trunc<vf><GPI:mode>2" > + [(set (match_operand:GPI 0 "register_operand" "=?r,w") > + (FIXUORS:GPI (match_operand:<VF> 1 "register_operand" "w,w")))] > + "TARGET_FLOAT" > + "@ > + fcvtz<su>\t%<w>0, %<s>1 > + fcvtz<su>\t%<s>0, %<s>1" > + [(set_attr "type" "f_cvtf2i,neon_fp_to_int_s")] > +) So the point here is that we need to fork the pattern for two reasons. Before we were iterating over both floating-point modes as the input to any integer-modes as output. Because only the same-sized instructions have vector-register to vector-register forms we need two patterns. One for same-size, one for cross-size. And one more special pattern for HFmode. This makes sense to me. A comment explaining why we need the two patterns would be even easier to read. This pattern gives us SFmode to SImode and DFmode to DImode. > +(define_insn "<optab>_trunchf<GPI:mode>2" > + [(set (match_operand:GPI 0 "register_operand" "=r") > + (FIXUORS:GPI (match_operand:HF 1 "register_operand" "w")))] > + "TARGET_FP_F16INST" > + "fcvtz<su>\t%<w>0, %h1" > + [(set_attr "type" "f_cvtf2i")] This pattern we need for HFmode to SImode. > +(define_insn "<optab>_trunc<vgp><GPI:mode>2" > [(set (match_operand:GPI 0 "register_operand" "=r") > - (FIXUORS:GPI (match_operand:GPF_F16 1 "register_operand" "w")))] > + (FIXUORS:GPI (match_operand:<VGP> 1 "register_operand" "w")))] > "TARGET_FLOAT" > - "fcvtz<su>\t%<GPI:w>0, %<GPF_F16:s>1" > + "fcvtz<su>\t%<w>0, %<wv>1" > [(set_attr "type" "f_cvtf2i")] > ) And this pattern gives SFmode to DImode and DFmode to SImode. Comments would definitely help here. > diff --git a/gcc/config/aarch64/iterators.md b/gcc/config/aarch64/iterators.md > index cceb575..166a044 100644 > --- a/gcc/config/aarch64/iterators.md > +++ b/gcc/config/aarch64/iterators.md > @@ -391,6 +391,9 @@ > (define_mode_attr w1 [(HF "w") (SF "w") (DF "x")]) > (define_mode_attr w2 [(HF "x") (SF "x") (DF "w")]) > > +;; For inequal width float to int conversion > +(define_mode_attr wv [(DI "s") (SI "d")]) Could you invent a more descriptive name for this? > + > (define_mode_attr short_mask [(HI "65535") (QI "255")]) > > ;; For constraints used in scalar immediate vector moves > @@ -399,6 +402,14 @@ > ;; For doubling width of an integer mode > (define_mode_attr DWI [(QI "HI") (HI "SI") (SI "DI") (DI "TI")]) > > +(define_mode_attr vf [(SI "sf") (DI "df")]) > + > +(define_mode_attr VF [(SI "SF") (DI "DF")]) These two are fcvt_target and FCVT_TARGET ? > +(define_mode_attr vgp [(SI "df") (DI "sf")]) > + > +(define_mode_attr VGP [(SI "DF") (DI "SF")]) These names don't make sense to me - V is usually vector and GP sounds like general purpose. Maybe something like fcvt_change_mode ? Try to be more descriptive. > + > ;; For scalar usage of vector/FP registers > (define_mode_attr v [(QI "b") (HI "h") (SI "s") (DI "d") > (HF "h") (SF "s") (DF "d") > @@ -432,7 +443,7 @@ > (define_mode_attr vas [(DI "") (SI ".2s")]) > > ;; Map a floating point mode to the appropriate register name prefix This comment is out of date after your changes, please update it. > -(define_mode_attr s [(HF "h") (SF "s") (DF "d")]) > +(define_mode_attr s [(HF "h") (SF "s") (DF "d") (SI "s") (DI "d")]) > > ;; Give the length suffix letter for a sign- or zero-extension. > (define_mode_attr size [(QI "b") (HI "h") (SI "w")]) Thanks, James
