[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: include/llvm/CodeGen/MachineFunction.h:332
   bool HasEHFunclets = false;
+  bool HasSEHFunclets = false;
 
----------------
mgrang wrote:
> rnk wrote:
> > IMO this should really live on the AArch64-specific function info, since it's not general to x64/x86 yet.
> > 
> > Also, this isn't SEH specific, this is set to true only by localescape. Maybe you should rename it? localescape was intended to be orthogonal to SEH, potentially usable for non-externally visible lambdas, although the codegen right now is particularly bad.
> I tried to move this to AArch64MachineFunctionInfo. I set it in AArch64RegisterInfo when I encounter LOCAL_ESCAPE and need to access it in AArch64FrameLowering. But it seems setting it in AArch64RegisterInfo is too late. So for now I have kept it on the MF.
I see, we could really only do that if localescape was lowered in target dependent code. I think this is fine the way it is now. It's clearer (to me, at least) after the rename.


================
Comment at: lib/CodeGen/AsmPrinter/WinException.cpp:548-549
 
-  // Emit a label assignment with the SEH frame offset so we can use it for
-  // llvm.x86.seh.recoverfp.
-  StringRef FLinkageName =
-      GlobalValue::dropLLVMManglingEscape(MF->getFunction().getName());
-  MCSymbol *ParentFrameOffset =
-      Ctx.getOrCreateParentFrameOffsetSymbol(FLinkageName);
-  const MCExpr *MCOffset =
-      MCConstantExpr::create(FuncInfo.SEHSetFrameOffset, Ctx);
-  Asm->OutStreamer->EmitAssignment(ParentFrameOffset, MCOffset);
+  if (MF->getTarget().getTargetTriple().getArch() !=
+      Triple::ArchType::aarch64) {
+    // Emit a label assignment with the SEH frame offset so we can use it for
----------------
This can just be `isAArch64`, we already assigned that to a member boolean.


================
Comment at: lib/Target/AArch64/AArch64AsmPrinter.cpp:697
     break;
+    case AArch64::MOVMCSym: {
+    MCSymbol *Sym = MI->getOperand(1).getMCSymbol();
----------------
mgrang wrote:
> @rnk Yes, you are correct that the assert here will fail is the localrecover comes before localescape.
> I have tried to model this similar to X86 but the problem is AArch64 cannot do an ADRP on an MCSymbol. So I wasn't sure how to materialize this in the assembler.
> Could you give me some idea on how this can be done?
I guess I've been assuming that you could assemble something like this in a .s file for aarch64, like you can for x86:
```
main:
  movl $.Lasdf, %eax
  ret
.Lasdf = 1234
```

I would assume that the aarch64 equivalent would be:
```
main:
  mov d0, #.Lasdf
  ret
.Lasdf = 1234
```

But that doesn't assemble. Does gnu as support that? I don't have an aarch64 binutils build. Maybe llvm-mc should allow mov/movk to accept a symbolic label operand.

I don't think we would want to use ADRP, IIRC that's for materializing some PC-relative address, but obviously this isn't that kind of symbol.


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

https://reviews.llvm.org/D53540