diff mbox

[1/4] Using gen_int_mode instead of GEN_INT

Message ID 87y574mr2h.fsf@talisman.default
State New
Headers show

Commit Message

Richard Sandiford Sept. 10, 2013, 7:09 p.m. UTC 
James Greenhalgh <james.greenhalgh@arm.com> writes:
> This seems to have caused PR58373. The bug occurs where GEN_INT would
> previously have then been used to build a constant of vector mode.
>
> These pop in a few places when building for AArch64, though I did
> the debugging using gcc.target/aarch64/vect-fcm-eq-d.c
>
> Here we could get in to the situation where
> simplify_unary_expression_1 would see (V2DI: NOT (NEG X))
> and try to generate (V2DI: PLUS (X - 1)). In turn, we would reach the
> call in plus_constant to gen_int_mode (-1, v2di), followed by
> trunc_int_for_mode (-1, v2di) and this assert would trigger:
>
>    /* You want to truncate to a _what_?  */
>    gcc_assert (SCALAR_INT_MODE_P (mode));
>
> In this fix I catch the case where gen_int_mode has been asked to
> build a vector mode, and call trunc_int_for_mode on the inner mode
> of that vector. A similar fix could sit in trunc_int_for_mode if
> you would prefer.
>
> Bootstrapped on x86_64-unknown-linux-gnu with no issues and regression
> tested for aarch64-none-elf with the expected benefits and no regressions.

Sorry for the breakage.  gen_int_mode and GEN_INT really are only for
scalar integers though.  (So is plus_constant.)  Vector constants should
be CONST_VECTORs rather than CONST_INTs.

I think the gcc.target/aarch64/vect-fcm-eq-d.c failure is from a latent
bug in the way (neg (not ...)) and (not (neg ...)) are handled.
Could you give the attached patch a go?

I looked through the other calls to plus_constant in simplify-rtx.c,
but they seem OK.

Thanks,
Richard


gcc/
	* simplify-rtx.c (simplify_unary_operation_1): Use simplify_gen_binary
	for (not (neg ...)) and (neg (not ...)) casees.

Comments

James Greenhalgh Sept. 10, 2013, 9:38 p.m. UTC | #1 
On Tue, Sep 10, 2013 at 08:09:42PM +0100, Richard Sandiford wrote:
> Sorry for the breakage.  gen_int_mode and GEN_INT really are only for
> scalar integers though.  (So is plus_constant.)  Vector constants should
> be CONST_VECTORs rather than CONST_INTs.
> 
> I think the gcc.target/aarch64/vect-fcm-eq-d.c failure is from a latent
> bug in the way (neg (not ...)) and (not (neg ...)) are handled.
> Could you give the attached patch a go?

Thanks Richard, this patch fixes the test FAILs I was seeing.

Cheers,
James
Graham Stott Sept. 11, 2013, 12:02 p.m. UTC | #2 
Hi Richard,

There is some minor testsuite fallout with these patches on MIPS a
couple of tests (see below)ICE ingen_int_mode () in both these ICE the mode is CCmode.

Here we arrive at gen_int_mode () via a use of plus_constant () is cse.c

with a register with CCmode called from  find_reg_offset_for_const ().

The plus_constant () ends up trying to create the reg+const rtx via


  if (c != 0)
    x = gen_rtx_PLUS (mode, x, gen_int_mode (c, mode));


with the gen_int_mode 90 triggering the ICE.

It obviously doesn't make sensegenerating a reg + const where the

reg has CCMode.

I've only got results from check-gcc-c testsuite so far.


Graham


testsuite/c-c++-common/cilk-plus/AN/builtin_func_double.c
testsuite/gcc.c-torture/execute/921013-1.c
diff mbox

Patch

Index: gcc/simplify-rtx.c
===================================================================
--- gcc/simplify-rtx.c	2013-09-10 20:02:08.756091875 +0100
+++ gcc/simplify-rtx.c	2013-09-10 20:02:09.002093907 +0100
@@ -825,7 +825,8 @@  simplify_unary_operation_1 (enum rtx_cod
 
       /* Similarly, (not (neg X)) is (plus X -1).  */
       if (GET_CODE (op) == NEG)
-	return plus_constant (mode, XEXP (op, 0), -1);
+	return simplify_gen_binary (PLUS, mode, XEXP (op, 0),
+				    CONSTM1_RTX (mode));
 
       /* (not (xor X C)) for C constant is (xor X D) with D = ~C.  */
       if (GET_CODE (op) == XOR
@@ -932,7 +933,8 @@  simplify_unary_operation_1 (enum rtx_cod
 
       /* Similarly, (neg (not X)) is (plus X 1).  */
       if (GET_CODE (op) == NOT)
-	return plus_constant (mode, XEXP (op, 0), 1);
+	return simplify_gen_binary (PLUS, mode, XEXP (op, 0),
+				    CONST1_RTX (mode));
 
       /* (neg (minus X Y)) can become (minus Y X).  This transformation
 	 isn't safe for modes with signed zeros, since if X and Y are