[PATCH] D41723: Introduce the "retpoline" x86 mitigation technique for variant #2 of the speculative execution vulnerabilities disclosed today, specifically identified by CVE-2017-5715, "Branch Target Injection", and is one of the two halves to Spectre..

[Matthias Braun via Phabricator via llvm-commits]

MatzeB accepted this revision.
MatzeB added a comment.

LGTM, I'm all for getting this into the LLVM tree, we can fine tune later as necessary.

This is the first widely used machine module pass. Adding a module pass into the pipeline means we will have all the MI in memory at the same time rather than creating MI for 1 function at a time and freeing it after emitting each function. So it would be good to keep an eye on the compilers memory consumption... Maybe we can find a way to refactor the codegen pipeline later so that we can go back to 1 function at a time and still have a way to emit some new code afterwards...



================
Comment at: llvm/include/llvm/CodeGen/TargetPassConfig.h:412-413
 
+  /// This pass may be implemented by targets that want to run passes
+  /// that emit MI directly and bypass all other machine passes.
+  virtual void addEmitPass() {}
----------------
Even though this documentation is mostly copied, how about changing it to:
```
Targets may add passes immediately before machine code is emitted in this callback.
This is called later than addPreEmitPass().
```


================
Comment at: llvm/include/llvm/CodeGen/TargetPassConfig.h:414
+  /// that emit MI directly and bypass all other machine passes.
+  virtual void addEmitPass() {}
+
----------------
I find the name `addEmitPass()` misleading as it isn't adding the assembly emission passes. The best bad name I can think of right now is `addPreEmit2()`, in the spirit of the existing `addPreSched2()` callback...


================
Comment at: llvm/lib/Target/X86/X86RetpolineThunks.cpp:83-84
+  auto *TPC = getAnalysisIfAvailable<TargetPassConfig>();
+  if (!TPC)
+    return false;
+
----------------
There shouldn't be any way to use any Target/X86 without also having a TargetPassConfig in the pipeline. An assert should be enough.


Repository:
  rL LLVM

https://reviews.llvm.org/D41723