Re: [PATCH][ARM] Remove Thumb-2 iordi_not patterns

[Kyrill Tkachov <kyrylo dot tkachov at foss dot arm dot com>]

Hi Wilco,

On 04/09/17 20:56, Wilco Dijkstra wrote:
> Kyrill Tkachov wrote:
>
> > > After Bernd's change almost all DI mode instructions are split before register
> > > allocation. So instructions using DI mode no longer exist and thus these
> > > extend variants can never be matched and are thus redundant.
> > Bernd's patch splits them when we don't have NEON. When NEON is
> > available though
> > they still maintain the DImode so we'd still benefit from these
> > transformations, no?
> While you're right it may be possible to trigger these instructions, ORN is already
> so rare that it is hardly beneficial to have an instruction for it, and ORN of an extended
> value never ever happens. So there is absolutely no benefit in keeping these versions
> temporarily until we fix Neon too.

Well, it was deemed useful enough to be added in the first place. I'd 
rather not remove it
if it can still be used. For the extended value I use the testcase:

unsigned long long
foo (unsigned long long a, unsigned long b)
{
  return a | ~(unsigned long long) b;
}

I compile with -O2 -S -mthumb -mcpu=cortex-a57 and I get:
        orn     r0, r0, r2
        mov     r1, #-1
        bx      lr

with the *iordi_notzesidi_di pattern being present in the postreload RTL 
dump.
Bernd's patches don't split DImode operations for NEON so I think we 
should still get the same
codegen without them. So this shows an example where the pattern can be 
used.


> For the Neon case my proposal is to use the VFP early expansion (so you get an efficient
> expansion by default in all cases). You can then use -mneon-for-64bits to enable the use
> of Neon instructions (which may be even better in some cases). There are quite a few
> patches in this series already and more to come soon!

So I think we should first move the NEON expansion to use the early VFP 
expansion
(with proper evaluation) and then once that's in and we can guarantee 
that these patterns
can indeed never match (due to having been expanded as SImode operations 
early on) we can remove
these patterns.

So I think this is overall ok, but we need to enable the early expansion 
everywhere first before going ahead
with this patch.

Thanks,
Kyrill

> Wilco