On Fri, Jun 16, 2017 at 11:07:41AM +0100, Kyrill Tkachov wrote: > > On 16/06/17 10:07, James Greenhalgh wrote: > >On Wed, Jun 14, 2017 at 11:21:30AM +0100, Kyrill Tkachov wrote: > > > > <...> > > > >>That movv2di expander is the one in vec-common.md that ends up calling > >>neon_make_constant. I wonder why const0_rtx passed its predicate check > >>(that would require a V2DImode vector of zeroes rather than a const0_rtx). > >>Perhaps the midend code at this point doesn't check the operand predicate. > >> > >>In the builtin expansion code that you quoted I wonder wonder if we could > >>fail > >>more gracefully by returning CONST0_RTX (mode[argc]) to match the expected > >>mode of the operand (we've already emitted an error, so we shouldn't care > >>what RTL we emit as long as it doesn't cause an ICE). > > <...> > > > >>diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c > >>index e503891..b8d59c6 100644 > >>--- a/gcc/config/arm/arm.c > >>+++ b/gcc/config/arm/arm.c > >>@@ -12124,6 +12124,11 @@ neon_make_constant (rtx vals) > >> if (n_const == n_elts) > >> const_vec = gen_rtx_CONST_VECTOR (mode, XVEC (vals, 0)); > >> } > >>+ else if (vals == const0_rtx) > >>+ /* Something invalid, perhaps from expanding an intrinsic > >>+ which requires a constant argument, where a variable argument > >>+ was passed. */ > >>+ return const0_rtx; > >> else > >> gcc_unreachable (); > >> > >>I'm not a fan of this as the function has a precondition that its argument > >>is > >>a PARALLEL or a CONST_VECTOR and special-casing const0_rtx breaks that. I'd > >>rather we tried fixing this closer to the error source. Can you try the > >>suggestion above instead please? > >Your suggestion doesn't quite work, but this is pretty close to it. Rather > >than try to guess at the correct mode for CONST0_RTX (we can't just use > >mode[argc] as that will get you the scalar mode), we can just return target > >directly. That will ensure we've given something valid back in the correct > >mode, even if it is not all that useful. > > Yeah, that actually looks better. > > >Bootstrapped on arm-none-linux-gnueabihf. OK? > > Ok.
Thanks. The patch applies cleanly to gcc-7-branch and gcc-6-branch, both of which I've bootstrapped and tested on arm-none-linux-gnueabihf without issue. Is it OK for me to apply these backports and close out the PR (it is marked as a 6/7 regression). Thanks, James > >--- > >gcc/ > > > >2017-06-15 James Greenhalgh <james.greenha...@arm.com> > > > > PR target/71778 > > * config/arm/arm-builtins.c (arm_expand_builtin_args): Return TARGET > > if given a non-constant argument for an intrinsic which requires a > > constant. > > > >gcc/testsuite/ > > > >2017-06-15 James Greenhalgh <james.greenha...@arm.com> > > > > PR target/71778 > > * gcc.target/arm/pr71778.c: New. > > >
