[PATCH] D50288: [ARM64] [Windows] Exception handling, part #2
Francis Visoiu Mistrih via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Sep 3 06:43:43 PDT 2018
thegameg added inline comments.
================
Comment at: lib/Target/AArch64/AArch64FrameLowering.cpp:159
+static bool isSEHOpcode(unsigned Opc) {
+ switch (Opc) {
----------------
Should this be in `AArch64InstrInfo`?
================
Comment at: lib/Target/AArch64/AArch64FrameLowering.cpp:484
+ assert(false && "No SEH Opcode for this instruction");
+ case AArch64::STPDpre:
+ case AArch64::LDPDpost: {
----------------
How about:
```
case AArch64::STPDpost:
Imm = -Imm;
LLVM_FALLTHROUGH
case AArch64::STPDpre:
[...]
```
Same applied to the other cases below.
================
Comment at: lib/Target/AArch64/AArch64FrameLowering.cpp:491
+ MIB = BuildMI(MF, DL, TII.get(AArch64::SEH_SaveFRegP_X))
+ .addImm(Reg0 - AArch64::D0)
+ .addImm(Reg1 - AArch64::D0)
----------------
Is `Reg0 - AArch64::D0` the same as `MRI->getDwarfRegNum`?
Same applied to the other cases below.
================
Comment at: lib/Target/AArch64/AArch64FrameLowering.cpp:508
+ .setMIFlag(Flag);
+ else {
+ MIB = BuildMI(MF, DL, TII.get(AArch64::SEH_SaveRegP_X))
----------------
Why the extra `{` / `}`?
================
Comment at: lib/Target/AArch64/AArch64FrameLowering.cpp:1440
+static bool invalidateWindowsRegisterPairing(unsigned Reg1, unsigned Reg2,
+ bool NeedsWinCFI) {
+ // If we are generating register pairs for a Windows function that requires
----------------
Indentation seems off.
================
Comment at: lib/Target/AArch64/AArch64FrameLowering.cpp:1629
+ // Windows unwinding codes require that gprs be consecutive if they are paired.
+ if (NeedsWinCFI) {
+ MIB.addReg(Reg1, getPrologueDeath(MF, Reg1));
----------------
I would rather just special-case the whole block since it seems to be having a pretty different behaviour than the non-wincfi path, but it's just a personal preference.
================
Comment at: lib/Target/AArch64/AArch64FrameLowering.cpp:1706
+
+ if (NeedsWinCFI) {
+ MIB.addReg(Reg1, getDefRegState(true));
----------------
Same here.
Repository:
rL LLVM
https://reviews.llvm.org/D50288
More information about the llvm-commits
mailing list