[PATCH] D17736: [SSP, 1/2] Refactor to support customizable stack guard load from IR level.

[Tim Shen via llvm-commits]

timshen added inline comments.

================
Comment at: lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:5376
@@ +5375,3 @@
+    // SPDescriptor expects either an address, or an intrinsic call, so we need
+    // to strip the LoadInst if possible.
+    if (auto Load = dyn_cast<LoadInst>(I.getArgOperand(0))) {
----------------
iteratee wrote:
> I'm not sure I understand this comment.
Updated - simply forward the Value* to visitSPDescriptorParent, and inspect if it's a CallInst or a Load there.

================
Comment at: lib/CodeGen/SelectionDAG/SelectionDAGBuilder.h:550
@@ -550,3 +549,3 @@
 
-    /// The guard variable which we will compare against the stored value in the
-    /// stack protector stack slot.
+    /// It either stores an address to the stack guard, or stores a
+    /// llvm.experimental_stackguardvalue CallInst.
----------------
iteratee wrote:
> Is this still accurate? Are we keeping support for loading an address?
According to the documentation, we never support loading an arbitrary address. We only support loading @__stack_chk_guard. I would say in visitSPDescriptorParent, it doesn't have to check if the LoadInst is actually loading @__stack_chk_guard, because we are already doing more than that (loads @__guard_local on OpenBSD).


http://reviews.llvm.org/D17736