[PATCH][gcse] PR rtl-optimization/69886: Check target mode in can_assign_to_reg_without_clobbers_p

[Richard Biener]

On Wed, Feb 24, 2016 at 10:34 AM, Kyrill Tkachov
<kyrylo.tkachov@foss.arm.com> wrote:
> Hi all,
>
> In this PR we get an ICE when the hoist pass ends up creating an
> (insn 88 0 0 (set (reg:OI 136)
>         (const_int 0 [0])) -1
>      (nil))
>
> instruction. AArch64 doesn't support such an OImode set.
> The only OImode set operations that aarch64 supports are load/store-multiple
> operations
> on vector registers.
>
> want_to_gcse_p should have rejected this move long before
> process_insert_insn tried to
> insert it in the stream.  But it didn't because
> can_assign_to_reg_without_clobbers_p is only
> given the (const_int 0) expression and asked whether there can be a valid
> SET operation on that.
> It should also consider the mode that such an operation is requested in,
> rather than extracting
> the mode from the operand (VOIDmode for CONST_INTs). Luckily, want_to_gcse_p
> already has a mode
> argument that it uses in its costs calculation, so we can just pass it down.
>
> This patch extends can_assign_to_reg_without_clobbers_p to take a mode
> argument and use it when
> testing the validity of the SET instructions that it creates, so such an
> OImode move is properly
> rejected.
>
> Bootstrapped and tested on aarch64-none-linux-gnu, arm-none-linux-gnueabihf,
> x86_64-unknown-linux-gnu.
> There are no codegen differences on SPEC2006 for aarch64 resulting from this
> patch.
>
> This bug appears in all versions that have aarch64, so it's not a
> regression, but I think it's
> a fairly low risk patch.
>
> Is this ok for trunk now or when stage 1 reopens?

Ok for trunk.

Thanks,
Richard.

> Thanks,
> Kyrill
>
> 2016-02-24  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>
>
>     PR rtl-optimization/69886
>     * gcse.c (can_assign_to_reg_without_clobbers_p): Accept mode
>     argument.  Use it when checking validity of set instructions.
>     (want_to_gcse_p): Pass mode to can_assign_to_reg_without_clobbers_p.
>     (compute_ld_motion_mems): Update can_assign_to_reg_without_clobbers_p
>     callsite.
>     * rtl.h (can_assign_to_reg_without_clobbers_p): Update prototype.
>     * store-motion.c (find_moveable_store): Update
>     can_assign_to_reg_without_clobbers_p callsite.
>
> 2016-02-24  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>
>
>     PR rtl-optimization/69886
>     * gcc.dg/torture/pr69886.c: New test.