[PATCH] D53540: [COFF, ARM64] Implement support for SEH extensions __try/__except/__finally

[Reid Kleckner via Phabricator via llvm-commits]

rnk added inline comments.


================
Comment at: lib/Target/AArch64/AArch64ISelLowering.cpp:2744
+    else // This function handles the SP or FP case.
+      Reg = RegInfo->getFrameRegister(MF);
+    return DAG.getCopyFromReg(DAG.getEntryNode(), dl, Reg,
----------------
smeenai wrote:
> rnk wrote:
> > efriedma wrote:
> > > Are you sure this does the right thing in general?  The link between the choice of register here, and the register used for the getFrameIndexReference call in WinException::emitEHRegistrationOffsetLabel , seems fragile at best.
> > It is fragile, but it's what was done for x86. Unfortunately, it's actually really hard to know which physical register is going to be used to address locals during ISel, so I just did this brittle thing and moved on. See these comments from AArch64FrameLowering::hasFP for some of the phase ordering problems:
> >   // With large callframes around we may need to use FP to access the scavenging
> >   // emergency spillslot.
> >   //
> >   // Unfortunately some calls to hasFP() like machine verifier ->
> >   // getReservedReg() -> hasFP in the middle of global isel are too early
> >   // to know the max call frame size. Hopefully conservatively returning "true"
> >   // in those cases is fine.
> >   // DefaultSafeSPDisplacement is fine as we only emergency spill GP regs.
> >   if (!MFI.isMaxCallFrameSizeComputed() ||
> >       MFI.getMaxCallFrameSize() > DefaultSafeSPDisplacement)
> >     return true;
> > 
> > I think we are mostly safe because when using EH funclets, an FP is always needed, so it's just a decision between "base" and "frame" pointer registers. That is currently entirely determined by the existence of user allocas with alignments above the standard ABI stack alignment.
> > 
> > However, you can imagine that some later pass might try to create more highly aligned stack objects, which would require a base pointer. For example, the register allocator might spill vector registers that require more stack alignment than the ABI provides. Today, for x86 at least, LLVM punts on this problem, and simply uses unaligned instructions for the spill and reload. I don't know how to get into the analogous situation for AArch64 or what LLVM would do, though.
> Is this at all related to https://bugs.llvm.org/show_bug.cgi?id=37169?
I had forgotten about that issue, but that's exactly the case I was describing. I proposed this solution there:

> I think localaddress should really lower down to a LEA of a zero-sized stack object. It's really trying to say, give me the address of the local variable area. The localrecover label assignments would instead compute the difference between this particular zero-sized stack object and their stack object.

For AArch64, LEA would really be ADC, or whatever is generated to materialize the address of an alloca. Does something like that make sense?

It occurs to me that this localescape system is pretty flawed, since not all local variables are static allocas. This source crashes clang today:

```
struct Foo {
  int x, y, z;
};
int filter(int);
void may_throw();
void seh_byval(Foo o) {
  __try {
    may_throw();
  } __except(filter(o.x)) {
  }
}
```

Oops. =/


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D53540/new/

https://reviews.llvm.org/D53540