On Mon, Feb 04, 2019 at 04:23:32AM -0600, Kyrill Tkachov wrote: > Hi all, > > Richard raised a concern about the RTL we use to represent the AdvSIMD SABD > (vector signed absolute difference) instruction. > We currently represent it as ABS (MINUS op1 op2). > > This isn't exactly what SABD does. ABS treats its input as a signed value > and returns the absolute of that. > > For example: > (sabd:QI 64 -128) == 192 (unsigned) aka -64 (signed) > whereas > (minus:QI 64 -128) == 192 (unsigned) aka -64 (signed), (abs ...) of that is > 64. > > A better way to describe the instruction is with MINUS (SMAX (op1 op2) SMIN > (op1 op2)). > This patch implements that, and also implements similar semantics for the > UABD instruction > that uses UMAX and UMIN. > > That way for the example above we'll have: > (minus:QI (smax:QI (64 -128)) (smin:QI (64 -128))) == (minus:QI 64 -128) == > 192 (or -64 signed) which matches > what SABD does. > > Bootstrapped and tested on aarch64-none-linux-gnu. > > Ok for trunk?
Not without a comment explaining the above subtlety and preferably a testcase which would fail today on trunk. Otherwise, OK. James > > Thanks, > Kyrill > > 2019-04-02 Kyrylo Tkachov <kyrylo.tkac...@arm.com> > > * config/aarch64/iterators.md (max_opp): New code_attr. > (USMAX): New code iterator. > * config/aarch64/predicates.md (aarch64_smin): New predicate. > (aarch64_smax): Likewise. > * config/aarch64/aarch64-simd.md (abd<mode>_3): Rename to... > (*aarch64_<su>abd<mode>_3): ... Change RTL representation to > MINUS (MAX MIN). > > 2019-04-02 Kyrylo Tkachov <kyrylo.tkac...@arm.com> > > * gcc.target/aarch64/abd_1.c: New test.
