On Wed, Nov 22, 2017 at 12:15 PM, Richard Sandiford <richard.sandif...@linaro.org> wrote: > Jakub Jelinek <ja...@redhat.com> writes: >> On Wed, Nov 22, 2017 at 10:09:08AM +0000, Richard Sandiford wrote: >>> This patch replaces the REDUC_*_EXPR tree codes with internal functions. >>> This is needed so that the support for in-order reductions can also use >>> internal functions without too much complication. >>> >>> This came out of the review for: >>> https://gcc.gnu.org/ml/gcc-patches/2017-11/msg01516.html >>> >>> Tested on aarch64-linux-gnu, x86_64-linux-gnu and powerpc64le-linux-gnu. >>> OK to install? >> >> Wouldn't it be better to just have IFN_REDUC that takes as an additional >> argument INTEGER_CST with tree_code of the operation (so REDUC_MAX_EXPR >> would be transformed into REDUC (MAX_EXPR, ...) etc.)? >> That way we wouldn't need to add further internal fns if we want say >> multiplication reduction, or some other. > > I think it depends how we use them. The functions added here map > directly to optabs, so we'd only add a new one if we also added a > new optab. If there's no optab, or if there is an optab but the > target doesn't support it, then we open-code the reduction during > vectorisation. (That open-coding already happens for MULT, AND, IOR > and XOR, which have no optabs, although one of the SVE patches does > add optabs for the last three.) > > I think having separate functions makes sense in that case, since it > makes the mapping to optabs easier, and makes it easier to probe > for target support. Maybe an IFN_REDUC would be useful if we wanted > to defer the open-coding of other reductions past vectorisation, > but I'm not sure off-hand how useful that would be. E.g. we'd still > need to try to cost the eventual expansion when deciding profitability.
I also agree that separate IFNs map better to what we do (encode a target specific insn). Thus the patch is ok. Thanks, Richard. > Thanks, > Richard
