Re: [GCC][PATCH][mid-end] Optimize x * copysign (1.0, y) [Patch (1/2)]

[Richard Biener <rguenther@suse.de>]

On Mon, 12 Jun 2017, Tamar Christina wrote:

> Hi All,
> 
> this patch implements a optimization rewriting
> 
> x * copysign (1.0, y) and 
> x * copysign (-1.0, y) 
> 
> to:
> 
> x ^ (y & (1 << sign_bit_position))
> 
> This is done by creating a special builtin during matching and generate the
> appropriate instructions during expand. This new builtin is called XORSIGN.
> 
> The expansion of xorsign depends on if the backend has an appropriate optab
> available. If this is not the case then we use a modified version of the existing
> copysign which does not take the abs value of the first argument as a fall back.
> 
> This patch is a revival of a previous patch
> https://gcc.gnu.org/ml/gcc-patches/2015-10/msg00069.html
> 
> Bootstrapped on both aarch64-none-linux-gnu and x86_64 with no issues.
> Regression done on aarch64-none-linux-gnu and no regressions.
> 
> Ok for trunk?

Without looking at the patch a few comments.

First, nowadays please add an internal function instead of builtins.
You can even take advantage of Richards work to directly tie those
to optabs (he might want to chime in to tell you how).  You don't need
the fortran FE changes in that case.

Second, I think we should canonicalize copysign (-CST, x) to
copysign (CST, x), thus positive real constants.  You should be
able to use

(simplify
 (copysign negate_expr_p@0 @1)
 (copysign @0 @1))

to catch both the (negate @0) and REAL_CST case.

What does

>       (copysigns @0 (negate @1)): Likewise.

do?

Third, new IL that is present throughout the compilation always
poses the risk that while passes may be able to handle copysign
they do not handle xorsign (vectorization?).  In this case it
looks like the matching is simply to enhance RTL expansion
which means it should ideally be done close to RTL expansion
only.  If you write

(match (xorsign_p @0 @1)
 (mult:c (copysign real_onep @0) @1))

you can call gimple_xorsign_p (you need to declare it, see
the generated gimple-match.c file for the definition) from,
say, pass_optimize_widening_mul, which despite its name
is used as a kitchen-sink for late GIMPLE pattern-matching
stuff to enhance RTL expansion / instruction selection.

Thanks,
Richard.

> 
> gcc/
> 2017-06-07  Tamar Christina  <tamar.christina@arm.com>
> 
> 	* builtins.def (BUILT_IN_XORSIGN, BUILT_IN_XORSIGNF): New.
> 	(BUILT_IN_XORSIGNL, BUILT_IN_XORSIGN_FLOAT_NX): Likewise.
> 	* match.pd (mult (COPYSIGN:s real_onep @0) @1): New simplifier.
> 	(mult (COPYSIGN:s real_mus_onep @0) @1): Likewise.
> 	(copysigns @0 (negate @1)): Likewise.
> 	* builtins.c (expand_builtin_copysign): Promoted local to argument.
> 	(expand_builtin): Added CASE_FLT_FN_FLOATN_NX (BUILT_IN_XORSIGN) and
> 	CASE_FLT_FN (BUILT_IN_XORSIGN).
> 	(BUILT_IN_COPYSIGN): Updated function call.
> 	* optabs.h (expand_copysign): New bool.
> 	(expand_xorsign): New.
> 	* optabs.def (xorsign_optab): New.
> 	* optabs.c (expand_copysign): New parameter.
> 	* fortran/f95-lang.c (xorsignl, xorsign, xorsignf): New.
> 	* fortran/mathbuiltins.def (XORSIGN): New.
> 
> gcc/testsuite/
> 2017-06-07  Tamar Christina  <tamar.christina@arm.com>
> 
> 	* gcc.dg/tree-ssa/xorsign.c: New.
> 	* gcc.dg/xorsign_exec.c: New.
> 	* gcc.dg/vec-xorsign_exec.c: New.
> 	* gcc.dg/tree-ssa/reassoc-39.c (f2, f3): Updated constant to 2.

-- 
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)