Hi!
On Mon, Apr 27, 2020 at 05:30:24PM -0500, Peter Bergner wrote:
> rtl-optimization: ICE on testsuite/gcc.dg/sso/t5.c with -mcpu=future -mpcrel
> -O1 [PR94740]
>
> We ICE on the test case below because decompose_normal_address() doesn't
> expect to see memory operands with constant addresses like below without
> a (const:DI ...) wrapped around the PLUS:
>
> (mem/c:SI (plus:DI (symbol_ref:DI ("*.LANCHOR0") [flags 0x182])
> (const_int 4 [0x4])) [1 array+4 S4 A32])
>
> What we expect to see is:
>
> (mem/c:SI (const:DI (plus:DI (symbol_ref:DI ("*.LANCHOR0") [flags
> 0x182])
> (const_int 4 [0x4]))) [1 arrayD.2903+4 S4
> A32])
>
> Sometimes, combine adds the (const: ...) for us via simplify_binary_operand
> call, but that only happens when combine actually does something with this
> insn. The bad address from the test case actually comes from CSE, so the
> fix here is to make CSE add the (const: ...) whenever it creates a MEM with
> a constant address.
So, is it a rule that we always should have CONST here? Where is that
documented?
> gcc/
> PR rtl-optimization/94740
> * cse.c (cse_process_notes):
(Missing changelog entry here).
> diff --git a/gcc/cse.c b/gcc/cse.c
> index 5aaba8d80e0..cbe9e0a0692 100644
> --- a/gcc/cse.c
> +++ b/gcc/cse.c
> @@ -6328,6 +6328,14 @@ cse_process_notes (rtx x, rtx object, bool *changed)
> rtx new_rtx = cse_process_notes_1 (x, object, changed);
> if (new_rtx != x)
> *changed = true;
> + if (*changed && object != NULL_RTX && MEM_P (object))
> + {
> + /* Call simplify_rtx on the updated address in case it is now
> + a constant and needs to be wrapped with a (const: ...). */
> + rtx simplified_rtx = simplify_rtx (new_rtx);
> + if (simplified_rtx)
> + new_rtx = simplified_rtx;
> + }
> return new_rtx;
> }
You can just do
return simplify_replace_rtx (new_rtx, 0, 0);
(it doesn't return 0 if nothing changed, unlike many other simplify*
things) for everything inside the if (); but you can even lose the if ()
itself?
(Although, maybe you're doing this all somewhere else now? :-) )
> diff --git a/gcc/testsuite/gcc.target/powerpc/pr94740.c
> b/gcc/testsuite/gcc.target/powerpc/pr94740.c
> new file mode 100644
> index 00000000000..78e40fc84da
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/powerpc/pr94740.c
> @@ -0,0 +1,11 @@
> +/* PR rtl-optimization/94740 */
> +/* { dg-do compile } */
> +/* { dg-require-effective-target powerpc_future_ok } */
> +/* { dg-options "-O2 -mdejagnu-cpu=future -mpcrel" } */
> +
> +int array[8];
> +int
> +foo (void)
> +{
> + return __builtin_bswap32 (array[1]);
> +}
That part is okay for trunk, of course :-)
Segher
