[PATCH] D98200: [GlobalISel] Port over the SelectionDAG stack protector codegen feature.

[Jessica Paquette via Phabricator via llvm-commits]

paquette added inline comments.


================
Comment at: llvm/include/llvm/CodeGen/CodeGenCommonISel.h:9
+//
+// This file delcares common utilies that are shared between SelectionDAG and
+// GlobalISel frameworks.
----------------



================
Comment at: llvm/include/llvm/CodeGen/CodeGenCommonISel.h:23
+class MachineBasicBlock;
+/// A class which encapsulates all of the information needed to generate a
+/// stack protector check and signals to isel via its state being initialized
----------------
Nit: Not necessary to say this is a class; we know that from the code.


================
Comment at: llvm/include/llvm/CodeGen/CodeGenCommonISel.h:24
+/// A class which encapsulates all of the information needed to generate a
+/// stack protector check and signals to isel via its state being initialized
+/// that a stack protector needs to be generated.
----------------
This sentence is kind of a run-on, can you break it down a little?


================
Comment at: llvm/include/llvm/CodeGen/CodeGenCommonISel.h:33
+///
+/// Previously, generation of stack protectors was done exclusively in the
+/// pre-SelectionDAG Codegen LLVM IR Pass "Stack Protector". This necessitated
----------------
Nit: kill passive voice

> Previously, the "stack protector" pre-SelectionDAG IR pass handled stack protector generation.


================
Comment at: llvm/include/llvm/CodeGen/CodeGenCommonISel.h:42
+///
+/// Then it was noticed that since the sibling call optimization causes the
+/// callee to reuse the caller's stack, if we could delay the generation of
----------------



================
Comment at: llvm/include/llvm/CodeGen/CodeGenCommonISel.h:78
+///
+///   1. While one can not handle multiple IR level basic blocks at the
+///      SelectionDAG Level, one can generate multiple machine basic blocks
----------------



================
Comment at: llvm/include/llvm/CodeGen/CodeGenCommonISel.h:113
+///        class (which involves stashing information/creating the success
+///        mbbb and the failure mbb if we have not created one for this
+///        function yet) and export the guard variable that we are going to
----------------



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98200