[PATCH] D129693: [ARM] Don't emit Arm speculation hardening thunks under Thumb and vice-versa

[Kristof Beyls via Phabricator via llvm-commits]

kristof.beyls added a comment.

My apologies for missing this outstanding review for so long.



================
Comment at: llvm/include/llvm/CodeGen/IndirectThunks.h:28
 protected:
-  bool InsertedThunks;
+  unsigned InsertedThunks;
   void doInitialization(Module &M) {}
----------------
DavidSpickett wrote:
> Can we document what this contains? Now that it's more than did or did not insert something.
This ThunkInserter is a base class combining the shared logic needed to insert thunks for a few more specific transformation passes.

Exactly what needs to be tracked for "InsertedThunks" with this patch starts to depend on the more specific transformation pass. IIUC, with this pass what needs to be tracked is:
- For thunk-inserting transformation passes that target 32-bit Arm: whether A32 vs T32 thunk variants have been inserted.
- For thunk-inserting transformation passes for other architectures: whether thunks relevant for that transformation pass has been inserted.

Given that what needs to be tracked is becoming depending on the class deriving from this base class, I wonder if it wouldn't be better to get the type needed for InsertedThunks from the Derived class?
Either by having an additional template argument, e.g. `template <typename Derived, typename InsertedThunksType> class ThunkInserter`.
Or by requiring there to be a type in the derived class, e.g. `Derived::InsertedThunksType InsertedThunks`?


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

https://reviews.llvm.org/D129693