On 29/05/2019 15:47, Srinath Parvathaneni wrote:
Hello,
The initial implementation of the FP16 extension added HFmode support to
a limited number of the standard names. Following
https://gcc.gnu.org/ml/gcc-patches/2016-08/msg00168.html , this patch
extends the HFmode support to the names implemented by the ARM
<vrint_pattern> and l<vrint_pattern> expanders: btrunc, ceil, round,
floor, nearbyint and rint. This patch also changes the patterns
supporting the neon_vrnd* and neon_vcvt* Adv.SIMD intrinsics to use the
standard names, where apropriate.
No new tests are added. The ARM tests for the SF and DF mode variants of
these names are based on GCC builtin functions and there doesn't seem to
be an obvious alternative to trigger the new patterns through the
standard names. The pattern definitions though are tested through the
Adv.SIMD intrinsics.
The following patch reworks the implementation for HFmode VRINT to
remove a number of redundant constructs that were added in the initial
implementation.
The two patches have been tested for arm-none-linux-gnueabihf with
native bootstrap and make check and for arm-none-eabi with
cross-compiled check-gcc on an ARMv8.4-A emulator.
Ok for trunk? If ok, could someone please commit the patch on my behalf,
I don't have commit rights.
2019-05-29 Srinath Parvathaneni <srinath.parvathan...@arm.com>
Matthew Wahab <matthew.wa...@arm.com>
* config/arm/iterators.md (fp16_rnd_str): Replace UNSPEC_VRND
values with equivalent UNSPEC_VRINT values. Add UNSPEC_NVRINTZ,
UNSPEC_NVRINTA, UNSPEC_NVRINTM, UNSPEC_NVRINTN, UNSPEC_NVRINTP,
UNSPEC_NVRINTX.
(vrint_variant): Fix some white-space.
(vrint_predicable): Fix some white-space.
* config/arm/neon.md (neon_v<fp16_rnd_str><mode>): Replace
FP16_RND iterator with NEON_VRINT and update the rest of the
pattern accordingly.
(neon_vcvt<vcvth_op><sup><mode>): Replace with
neon_vcvt<nvrint_variant><su><mode>.
(neon_vcvt<nvrint_variant><su><mode>): New.
(neon_vcvtn<su><mode>): New.
* config/arm/unspecs.md: Add UNSPEC_VRINTN.
* config/arm/vfp.md (neon_v<fp16_rnd_str>hf): Convert to an
expander invoking <vrint_pattern>hf2.
(neon_vrndihf): Remove.
(neon_vrndnhf): New.
(neon_vcvt<vcvth_op>h<sup>si): Remove.
(<vrint_pattern>hf2): New.
(l<vrint_pattern><su_optab>hfsi2): New.
(neon_vcvt<vrint_variant>h<su>si): New.
(neon_vcvtnh<su>si): New.
Picking just one pattern as an example here, why does:
+(define_insn "l<vrint_pattern><su_optab>hfsi2"
+ [(set (match_operand:SI 0 "register_operand" "=t")
+ (FIXUORS:SI
+ (unspec:HF [(match_operand:HF 1 "register_operand" "t")] VCVT)))]
+ "TARGET_VFP_FP16INST"
+ "vcvt<vrint_variant>.<su>32.f16\\t%0, %1"
+ [(set_attr "predicable" "no")
+ (set_attr "conds" "unconditional")
+ (set_attr "type" "f_cvtf2i")]
+)
still need the unspec inside the FIXUORS? If the pattern does what the
FIX says, then it should be safe to do this directly on the register,
without wrapping it first in an UNSPEC.
The other thing I noticed was:
UNSPEC_VRINTA ; Represent a float to integral float rounding
; towards nearest, ties away from zero.
+ UNSPEC_VRINTN ; Represent a float to integral float rounding
+ ; towards nearest.
So we have VRINTA which rounds to nearest, but rounds ties away from
zero, but VRINTN rounds to nearest but doesn't say what happens when
there is a tie (ie a value exactly half-way between two results. The
architecture treats this as ties-to-even, so I think we should say that
here.
R.
