Re: Validity of SUBREG+AND-imm transformations

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

On 04/03/16 14:41, Kyrill Tkachov wrote:
> On 04/03/16 11:59, Segher Boessenkool wrote:
> > On Mon, Feb 29, 2016 at 10:51:24AM +0000, Kyrill Tkachov wrote:
> > > So I'm trying to create a define_insn to match something like:
> > >    [(set (match_operand:SI 0 "register_operand" "=r")
> > >      (ashift:SI
> > >        (match_operand:SI 1 "register_operand" "r")
> > >        (and:QI
> > >          (match_operand:QI 2 "register_operand" "r")
> > >          (match_operand:QI 3 "const_int_operand" "n"))))]
> > >
> > >
> > > where operands[3] is 31 for SImode. The 'and' expression has to be in
> > > QImode because our shift expanders
> > > expand the shift amount to QImode.
> > Is there any reason for that?  Why not SImode?
>
> It's been that way since the beginning. I don't know the reason.
> I tried changing to SImode. It requires a lot mechanical changes to
> all the shift+arithmetic patterns, but it does work.
> It catches a few more cases that my original approach (not sure why yet,
> would have to dig in the dumps) but it also widens a few memory accesses
> i.e. we'll now be loading a 32-bit value from memory instead of a byte.
> Overall it could be a better approach, though it would be a larger patch
> and would need more investigation of potential side effects.

Although there are case where we hit the same problem:
unsigned long
f3 (unsigned long bit_addr)
{
  unsigned long bitnumb = bit_addr & 63;
  return (1L << bitnumb);
}

combine will try to match:
(set (reg:DI 78)
    (ashift:DI (reg:DI 80)
        (subreg:SI (and:DI (reg:DI 0 x0 [ bit_addr ])
                (const_int 63 [0x3f])) 0)))

does that mean that the shift amount should be DImode?
Seems like a more flexible approach would be for the midend to be able to handle these things...

Kyrill


> Thanks,
> Kyrill
>
> > Segher