Alex Coplan <alex.cop...@arm.com> writes:
> Note that an obvious omission here is that this patch does not touch the
> mult patterns such as *add_<optab><ALLX:mode>_mult_<GPI:mode>. I found
> that I couldn't hit these patterns with C code since multiplications by
> powers of two always get turned into shifts by earlier RTL passes. If
> there's a way to reliably hit these patterns, then perhaps these should
> be updated as well.

Hmm.  Feels like we should either update them or delete them.  E.g.:

  *adds_<optab><mode>_multp2
  *subs_<optab><mode>_multp2

were added alongside the adds3.c and subs3.c tests that you're updating,
so if the tests don't/no longer need the multp2 patterns to pass,
there's a good chance that the patterns are redundant.

For reasons I never understood, the canonical representation is to use
(mult …) for powers of 2 inside a (mem …) but shifts outside of (mem …)s.
So perhaps the patterns were originally for address calculations that had
been moved outside of a (mem …) and not updated to shifts instead of mults.

AFAICT the full list of affected patterns is:

  *adds_<optab><mode>_multp2
  *subs_<optab><mode>_multp2
  *add_<optab><ALLX:mode>_mult_<GPI:mode>
  *add_uxt<mode>_multp2
  *sub_uxt<mode>_multp2

Is that right?  If so, I think we should consider a follow-on patch
to delete them.

> Testing:
>  * New test which checks for the correct syntax in all updated
>    patterns (fails before and passes after the aarch64.md change).
>  * New test can be assembled by both GAS and llvm-mc following the
>    change.
>  * Bootstrapped and regtested on aarch64-none-linux-gnu.
>
> OK for master?

OK as-is if paired with a follow-on patch to delete the patterns above
(preapproved if it passes testing).  Also OK without a follow-on patch
if the fix is extended to the patterns above too (but the first option
is better :-)).

Thanks for taking the time to find a test for each pattern.

Richard

Reply via email to