Re: [PATCH, ARM] Don't copy uncopyable instructions in gcse.c

[Steven Bosscher <stevenb dot gcc at gmail dot com>]

On Wed, May 20, 2009 at 10:56 PM, Julian Brown <julian@codesourcery.com> wrote:
> Current trunk seems to be broken for arm-linux-gnueabi, when
> configuring for ARMv7-a/Thumb mode (i.e. with --with-arch=armv7-a
> --with-mode=thumb). The fault happens (during glibc build) when GCSE
> makes a copy of a thumb2.md "pic_load_dot_plus_four" instruction
> (during local copy-propagation).

So how does your patch make a difference?  local copy propagation does
not use hash_scan_set AFAIK.


> ChangeLog
>
> ? ?gcc/
> ? ?* gcse.c (target.h): Include.
> ? ?(hash_scan_set): Don't make copies of instructions the target deems
> ? ?uncopyable.
> ? ?* config/arm/arm.c (arm_cannot_copy_insn_p): Check we have an
> ? ?instruction.


>  arm_cannot_copy_insn_p (rtx insn)
>  {
> -  rtx pat = PATTERN (insn);
> +  rtx pat;
> +
> +  if (!INSN_P (insn))
> +    return FALSE;

I think this can only happen for implicit sets, otherwise there always
should be an insn in hash_scan_set.  So maybe

+	       && (!insn  || ! targetm.cannot_copy_insn_p (insn)))

instead (in both places where you add a call to the hook), just to
keep the hook clean (IMHO passing NULL to cannot_copy_insn_p makes no
sense, given the name of the hook...).

I think the patch with this change is OK for the trunk (but that's not
an approval, I can't approve it ;-)

But if your problem is really caused in local-cprop, then you really
are just papering over the real bug...

Ciao!
Steven