This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: Be stricter about CONST_VECTOR operands
- From: James Greenhalgh <james dot greenhalgh at arm dot com>
- To: <gcc-patches at gcc dot gnu dot org>, <richard dot sandiford at linaro dot org>
- Cc: <nd at arm dot com>
- Date: Mon, 6 Nov 2017 14:30:00 +0000
- Subject: Re: Be stricter about CONST_VECTOR operands
- Authentication-results: sourceware.org; auth=none
- Authentication-results: spf=pass (sender IP is 217.140.96.140) smtp.mailfrom=arm.com; linaro.org; dkim=none (message not signed) header.d=none;linaro.org; dmarc=bestguesspass action=none header.from=arm.com;
- Nodisclaimer: True
- References: <87inenydq8.fsf@linaro.org>
- Spamdiagnosticmetadata: NSPM
- Spamdiagnosticoutput: 1:99
On Mon, Nov 06, 2017 at 09:10:23AM +0000, Richard Sandiford wrote:
> The recent gen_vec_duplicate patches used CONST_VECTOR for all
> constants, but the documentation says:
>
> @findex const_vector
> @item (const_vector:@var{m} [@var{x0} @var{x1} @dots{}])
> Represents a vector constant. The square brackets stand for the vector
> containing the constant elements. @var{x0}, @var{x1} and so on are
> the @code{const_int}, @code{const_double} or @code{const_fixed} elements.
>
> Both the AArch32 and AArch64 ports relied on the elements having
> this form and would ICE if the element was something like a CONST
> instead. This showed up as a failure in vect-102.c for both arm-eabi
> and aarch64-elf (but not aarch64-linux-gnu, which is what the series
> was tested on).
>
> The two obvious options were to redefine CONST_VECTOR to accept all
> constants or make gen_vec_duplicate honour the existing documentation.
> It looks like other code also assumes that integer CONST_VECTORs contain
> CONST_INTs, so the patch does the latter.
>
> I deliberately didn't add an assert to gen_const_vec_duplicate
> because it looks like the SPU port *does* expect to be able to create
> CONST_VECTORs of symbolic constants.
>
> Also, I think the list above should include const_wide_int for vectors
> of TImode and wider.
>
> The new routine takes a mode for consistency with the generators,
> and because I think it does make sense to accept all constants for
> variable-length:
>
> (const (vec_duplicate ...))
>
> rather than have some rtxes for which we instead use:
>
> (vec_duplicate (const ...))
>
> Tested on aarch64-elf, aarch64-linux-gnu, x86_64-linux-gnu and
> powerpc64-linux-gnu. OK to install?
For what it is worth, I can see why this would fix the bug in the AArch64
port, and the patch looks good to me (though I can't approve it).
> (but not aarch64-linux-gnu, which is what the series was tested on).
Presumably it would fail there for -mcmodel=tiny too, we just don't run
that variant by default.
Thanks,
James
> 2017-11-06 Richard Sandiford <richard.sandiford@linaro.org>
>
> gcc/
> * doc/rtl.texi (const_vector): Say that elements can be
> const_wide_ints too.
> * emit-rtl.h (valid_for_const_vec_duplicate_p): Declare.
> * emit-rtl.c (valid_for_const_vec_duplicate_p): New function.
> (gen_vec_duplicate): Use it instead of CONSTANT_P.
> * optabs.c (expand_vector_broadcast): Likewise.