[PATCH] D55929: Initial AArch64 SLH implementation.

[Zola Bridges via Phabricator via llvm-commits]

zbrid added a comment.

Hi Kristof,

Thanks for writing this code! I had an easy time understanding what you were doing.

I was discussing this SLH implementation with Chandler and he suggested that it may be useful to write a design doc similar to the one for the x86 implementation, so other people in the community can understand and discuss the current/future design with the ARM specific details laid out. What do you think?



================
Comment at: llvm/trunk/lib/Target/AArch64/AArch64SpeculationHardening.cpp:411
+    // Only harden loaded values or addresses used in loads.
+    if (!MI.mayLoad())
+      continue;
----------------
Right now, this masks after every load in a program. Is one of the future optimizations you mention in the comment in this file to mask only after loads that are depended upon by later non-data invariant operations?


================
Comment at: llvm/trunk/lib/Target/AArch64/AArch64SpeculationHardening.cpp:447
+
+    if (HardenLoadedData)
+      for (auto Def : MI.defs()) {
----------------
I'm not sure if this case is possible, but is it possible that some defs are GPR and some aren't? If that's possible would it be worthwhile to harden the GPRs and non-GPRs in different ways? It appears to me that currently if any are non-GPR, then all of them are treated as non-GPRs.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55929/new/

https://reviews.llvm.org/D55929