This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [Patch ARM] Fix PR71778



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,
Kyrill


Thanks,
James

---
gcc/

2017-06-15  James Greenhalgh  <james.greenhalgh@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.greenhalgh@arm.com>

	PR target/71778
	* gcc.target/arm/pr71778.c: New.



Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]