On Wed, Jun 4, 2014 at 8:07 AM, Thomas Preud'homme
<thomas.preudho...@arm.com> wrote:
> Hi there,
>
> It seems from PR61320 that the bswap pass causes some problems when it
> replaces
> an OR expression by an unaligned access. Although it's not clear yet why the
> unaligned load does not go through the extract_bit_field codepath, it is
> necessary to
> provide a solution as this prevent sparc from bootstrapping. This patch takes
> the
> simple approach of cancelling the bswap optimization when the load that would
> replace the OR expression would be an unaligned load and the target has
> SLOW_UNALIGNED_ACCESS. In the long run this patch should be reverted as soon
> as the root cause of the current problem is found.
>
> The patch also rewrite the test to take into account the fact that the
> optimization is
> not done for some target. It also add some alignment hint so that more tests
> can be
> run even on STRICT_ALIGNMENT targets.
>
> ChangeLog changes are:
>
> *** gcc/ChangeLog ***
>
> 2014-06-03 Thomas Preud'homme <thomas.preudho...@arm.com>
>
> PR tree-optimization/61320
> * tree-ssa-math-opts.c (bswap_replace): Cancel bswap optimization when
> load is unaligned and would be slow for this target.
>
> *** gcc/testsuite/ChangeLog ***
>
> 2014-06-03 Thomas Preud'homme <thomas.preudho...@arm.com>
>
> * gcc.dg/optimize-bswaphi-1.c: Make variables global when possible to
> enforce correct alignment and make the test work better on
> STRICT_ALIGNMENT targets. Also adjust dg-final selectors when
> alignment
> cannot be controlled (read_*_3 ()).
> * gcc.dg/optimize-bswapsi-2.c: Likewise.
> * gcc.dg/optimize-bswapdi-3.c: Likewise.
>
> Bootstrapped on x86_64-linux-gnu and no regression found in the testsuite.
> Patch is
> in attachment. It applies on top of the one for PR61306 in the email titled
> "[PATCH] Fix PR61306: improve handling of sign and cast in bswap" but can be
> trivially modified to apply directly on trunk should that patch (PR61306)
> need to be
> improved.
>
> Is this ok for trunk?
I'd rather wait for the other bug to be fixed properly (given this patch
is ontop of that). Btw,
+ if (align < GET_MODE_ALIGNMENT (TYPE_MODE (load_type)) &&
+ SLOW_UNALIGNED_ACCESS (TYPE_MODE (load_type), align))
return false;
&& goes to the next line.
I too belive that presenting a wider possibly unaligned load to the expander
is good and gives it a chance to produce optimal code for the target.
I think that PR61320 is probably a dup of PR61306.
Still adding the align < GET_MODE_ALIGNMENT (TYPE_MODE (load_type))
condition is correct here, aligned loads are fine even in the bswap
code-path.
Richard.
> Best regards,
>
> Thomas
