[PATCH] D44824: [Spectre] Introduce a new pass to do speculative load hardening to mitigate Spectre variant #1 for x86.

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/lib/Target/X86/X86SpeculativeLoadHardening.cpp:1427
+  // FIXME: This hard codes a shift distance based on the number of bits needed
+  // to stay canonacial on 64-bit. We should compute this somehow and support
+  // 32-bit as part of that.
----------------
canonical*


================
Comment at: llvm/lib/Target/X86/X86SpeculativeLoadHardening.cpp:1437
+                 .addReg(TmpReg, RegState::Kill);
+  OrI->getOperand(OrInstEFLAGSOpIdx).setIsDead(true);
+  ++NumInstsInserted;
----------------
Maybe use

```
OrI->addRegisterDead(X86::EFLAGS, TRI);
```

It should scan the operands and find EFLAGS without needing magic numbers


================
Comment at: llvm/lib/Target/X86/X86SpeculativeLoadHardening.cpp:1760
+      ShiftInputReg = MRI->createVirtualRegister(&ShiftRC);
+      BuildMI(MBB, InsertPt, Loc, TII->get(X86::SUBREG_TO_REG), ShiftInputReg)
+          .addImm(0)
----------------
This should probably just be INSERT_SUBREG. SUBREG_TO_REG means you guarantee the upper bits are 0. But I'm not sure that's guaranteed here. And I don't think its really necessary.


Repository:
  rL LLVM

https://reviews.llvm.org/D44824