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

[Chandler Carruth via Phabricator via llvm-commits]

chandlerc added a comment.

Thanks, addressed.



================
Comment at: llvm/lib/Target/X86/X86SpeculativeLoadHardening.cpp:413
+    ++NumInstsInserted;
+    assert(ZeroI->getOperand(1).isImplicit() && ZeroI->getOperand(1).isDef() &&
+           "Must have an implicit def of EFLAGS!");
----------------
craig.topper wrote:
> should we also check getReg() == X86::EFLAGS in this assert.
Instead of this, I rewrote this to not hard-code `1` and to find the EFLAGS def operand and then do both the assert and setting it to dead.i


================
Comment at: llvm/lib/Target/X86/X86SpeculativeLoadHardening.cpp:1442
+  // 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.
----------------
craig.topper wrote:
> canonical is still mispelled
Doh! When the lines shifted I couldn't find it. Fixed for real. Along with some other typos in comments.


Repository:
  rL LLVM

https://reviews.llvm.org/D44824