llvm dev - Jun 2019 - How to handle ISD::STORE when both operands are FrameIndex?

On Tue, Jun 25, 2019 at 9:59 AM Tim Northover <t.p.northover at gmail.com>
wrote:
> On Tue, 25 Jun 2019 at 06:26, Gleb Popov via llvm-dev
> <llvm-dev at lists.llvm.org> wrote:
> >> While the store is being selected LLVM will just treat the value
being
> >> stored as a generic pointer-width integer unless you have written
a
> >> specific pattern for storing a FrameIndex somewhere.
> >
> > Actually, this is exactly my case. I have a pattern that looks like
> >
> > (store_stack IntRegs:$reg, FIOperand:$i)
> >
> > It worked for me when first operand was a register and now I stumbled
> upon two FrameIndicies.
>
> That looks more like it's storing *to* a FrameIndex than storing a
> FrameIndex, but it actually shouldn't matter. The same sequence of
> events I described would happen except the selected bare FrameIndex
> would feed into the pointer operand of the store rather than the value
> operand.
>
> If you had both kinds of pattern (storing an FI and storing to an FI)
> and wanted one to have priority you can add a "let AddedComplexity
> 10" or similar to one of them so that LLVM favours it when both would
> apply.
>
> >> FrameIndex values come from objects that are still allocated on
the
> stack at the time of the SDAG construction.  In your case it seems that the
> address and the value to store are both on the stack.  You don’t need to do
> anything in particular with it.  If you have a selection pattern of form
> “(store RegClassA:$Addr, RegClassB:$Val), (STORE $Addr, $Val)”, then it
> will force loading both, Addr and Val into registers.
> >
> > You mean, LLVM will automagically generate loads? This isn't
happening
> for me.
>
> "Materializing" would probably have been a less ambiguous choice
of
> words. I wouldn't expect a load instruction on most architectures.
>
> > And what's "STORE"? Is it somehow different from
"store"?
>
> On most targets in LLVM the implemented instruction names are written
> in capital letters (mostly): ADDrm, MULrr, STRXrs. The STORE there was
> meant to represent a target-specific store instruction (like you'd get
> on the RHS of a Pat instantiation) without committing to any
> particular architecture.
>
> Could you describe what *is* happening for you, BTW? Maybe with an
> "llc -debug" log on a simple example. We might be able to give
more
> specific advice with the actual error.
>
Sigh. I'm completely confused and lost. Thanks for bearing with me.

Here's my current definition:

def AddrFI: ComplexPattern<i32, 1, "SelectAddrFI", [frameindex],
[]>;

class StackAddress : CodePatPred<[{
  return cast<MemSDNode>(N)->getAddressSpace() == 1;
}]>;

class StoreFrag<SDPatternOperator op> : PatFrag <
  (ops node:$value, node:$ptr), (op node:$value,
node:$ptr)
>;
class StackStore <SDPatternOperator op> : StoreFrag <op>,
StackAddress;

def store_stack : StackStore<store>;

def StoreStackF : InstRI<2, (outs), (ins IntRegs:$reg, i32imm:$i),
                    "storestackf $reg, [$i]", [(store_stack i32:$reg,
AddrFI:$i)]>;

I'm puzzled why despite having "IntRegs:$reg" in ins list, it
still matches
FrameIndex:

SEL: Starting selection on root node: t14: ch = store<(store 4 into
%ir.p45, align 8, addrspace 1)> t10, FrameIndex:i32<2>,
FrameIndex:i32<3>,
undef:i32
ISEL: Starting pattern match
  Initial Opcode index to 4
  Morphed node: t14: ch = StoreStackF<Mem:(store 4 into %ir.p45, align 8,
addrspace 1)> FrameIndex:i32<2>, TargetFrameIndex:i32<3>, t10
ISEL: Match complete!
...
ISEL: Starting selection on root node: t11: i32 = FrameIndex<2>
ISEL: Starting pattern match
  Initial Opcode index to 0
  Match failed at index 0
LLVM ERROR: Cannot select: t11: i32 = FrameIndex<2>


> Cheers.
>
> Tim.
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20190626/f2213f95/attachment.html>

Hi Gleb,

On Wed, 26 Jun 2019 at 07:28, Gleb Popov <6yearold at gmail.com>
wrote:
> def StoreStackF : InstRI<2, (outs), (ins IntRegs:$reg, i32imm:$i),
>                     "storestackf $reg, [$i]", [(store_stack
i32:$reg, AddrFI:$i)]>;
>
> I'm puzzled why despite having "IntRegs:$reg" in ins list, it
still matches FrameIndex:
A register-class will match anything with the appropriate type (i32
here). The idea is that any value of that type can be put into a
register somehow, and matching whatever it happens to be (a FrameIndex
in this case) is exactly what's needed to do that.
> ISEL: Starting selection on root node: t11: i32 = FrameIndex<2>
> ISEL: Starting pattern match
>   Initial Opcode index to 0
>   Match failed at index 0
> LLVM ERROR: Cannot select: t11: i32 = FrameIndex<2>
Ah good, this is what I thought was probably happening so it's nothing
exotic. You need to be able to match a FrameIndex somehow. Not just
for this, but for something like:

    void foo() {
      int arr[5];
      bar(&arr[0]);
    }

It's pretty simple to implement matching, you generally just need to
convert it to some add instruction that can do SP+imm. See for example
lib/Target/AArch64/AArch64ISelDAGToDAG.cpp:2916.

Cheers.

Tim.

On Wed, Jun 26, 2019 at 12:38 PM Tim Northover <t.p.northover at
gmail.com>
wrote:
> Hi Gleb,
>
> On Wed, 26 Jun 2019 at 07:28, Gleb Popov <6yearold at gmail.com>
wrote:
> > def StoreStackF : InstRI<2, (outs), (ins IntRegs:$reg, i32imm:$i),
> >                     "storestackf $reg, [$i]", [(store_stack
i32:$reg,
> AddrFI:$i)]>;
> >
> > I'm puzzled why despite having "IntRegs:$reg" in ins
list, it still
> matches FrameIndex:
>
> A register-class will match anything with the appropriate type (i32
> here). The idea is that any value of that type can be put into a
> register somehow, and matching whatever it happens to be (a FrameIndex
> in this case) is exactly what's needed to do that.
>
That's a useful explanation, thanks.

> > ISEL: Starting selection on root node: t11: i32 = FrameIndex<2>
> > ISEL: Starting pattern match
> >   Initial Opcode index to 0
> >   Match failed at index 0
> > LLVM ERROR: Cannot select: t11: i32 = FrameIndex<2>
>
> Ah good, this is what I thought was probably happening so it's nothing
> exotic. You need to be able to match a FrameIndex somehow. Not just
> for this, but for something like:
>
>     void foo() {
>       int arr[5];
>       bar(&arr[0]);
>     }
>
> It's pretty simple to implement matching, you generally just need to
> convert it to some add instruction that can do SP+imm.

For the sake of simplicity I decided not to bother with stack register yet
and instead I just emit FrameIndex as immediate:

bool MyBackendDAGToDAGISel::SelectAddrFI(SDValue &N, SDValue &R) {
  if (N.getOpcode() != ISD::FrameIndex)
    return false;
  MachineFrameInfo &MFI = MF->getFrameInfo();
  int FX = cast<FrameIndexSDNode>(N)->getIndex();
  R = CurDAG->getTargetFrameIndex(FX, MVT::i32);
  return true;
}

This way I end up with

store %r1, [1]

and handle it in my CPU emulator accordingly.

So, instead of matching that FrameIndex in store, I really want to emit a
load first and then use a register in the store instruction.
Can you, please, advise me how to implement that?

See for example
> lib/Target/AArch64/AArch64ISelDAGToDAG.cpp:2916.
>
> Cheers.
>
> Tim.
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20190626/0b6684d8/attachment.html>