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

[Reid Kleckner via Phabricator via llvm-commits]

rnk added inline comments.


================
Comment at: lib/Target/AArch64/AArch64AsmPrinter.cpp:571
+    MCInst TmpInst;
+    TmpInst.setOpcode(AArch64::MOVKXi);
+    TmpInst.addOperand(MCOperand::createReg(MI->getOperand(0).getReg()));
----------------
mgrang wrote:
> I see that X86 generates LEA to calculate the address of the .L label. AArch64 does not have a similar instruction. I tried to use ADRP + ADD but that requires either a GlobalSymbol or an ExternalSymbol type. However, the .L label is of type MCSymbol.
> 
> So I generate a MOVK x1, #.Lfoo. The problem is that MOVK takes an unsigned immediate. The .L label here represents offset from FP. This works if the offset is positive. But for negative offsets we should generate a MOVN. However, I am not sure how to detect negative immediate without first generating a MOV.
> 
> Any suggestions?
I would expect you would want a generic MOVMCSym instruction to be able to materialize any label, not just ones that are known during assembly. This might get selected if we needed to materialize, for example, the address of the LSDA, which is also a private label.


================
Comment at: lib/Target/AArch64/AArch64ISelLowering.cpp:2731-2732
+  MCSymbol *OffsetSym =
+      MF.getMMI().getContext().getOrCreateParentFrameOffsetSymbol(
+          GlobalValue::dropLLVMManglingEscape(Fn->getName()));
+  SDValue OffsetSymVal = DAG.getMCSymbol(OffsetSym, PtrVT);
----------------
mgrang wrote:
> rnk wrote:
> > Yeah, this was a great hack. :)
> I am not really familiar with this piece of code. Could you please comment on whether this should work for AArch64?
I think it should. Fundamentally, we are trying to transfer information about one function's frame layout into an independent filter function. The idea is that we can write assembly like this:
```
foo:
.Lfoo = 0x48
  ...

foo$filt:
  mov r0, .Lfoo
```

Hm, if possible, it might be nice to codegen this whole thing after SD ISel. I wonder if there is some way to pass it through as an intrinsic and lower it later. That will avoid the whole concern of having a generic MCSymbol materialization instruction selection pattern.


================
Comment at: lib/Target/AArch64/AArch64ISelLowering.cpp:2729
+  // Get an MCSymbol that will ultimately resolve to the frame offset of the EH
+  // registration, or the .set_setframe offset.
+  MCSymbol *OffsetSym =
----------------
I think this is suppoosed to be ".seh_setframe offset"


https://reviews.llvm.org/D53540