[llvm] [CodeGen] Refactor and document ThunkInserter (PR #97468)

Kristof Beyls via llvm-commits llvm-commits at lists.llvm.org
Thu Jul 4 01:17:04 PDT 2024


================
@@ -183,15 +183,13 @@ static const struct ThunkNameAndReg {
 namespace {
 struct SLSBLRThunkInserter : ThunkInserter<SLSBLRThunkInserter> {
   const char *getThunkPrefix() { return SLSBLRNamePrefix; }
-  bool mayUseThunk(const MachineFunction &MF, bool InsertedThunks) {
-    if (InsertedThunks)
-      return false;
+  bool mayUseThunk(const MachineFunction &MF) {
+    // FIXME: ComdatThunks is only accumulated until the first thunk is created.
----------------
kbeyls wrote:

Right, with that longer explanation here, I think I understand.
I'd say that it is fine to use more than one line to explain a FIXME.

IIUC, this means that if a comdat thunk was emitted because one function didn't request a no-comdat; and later another function requires a "no-comdat" thunk to be emitted, this can not be done.

I don't fully remember why we had to introduce the "NoComdat" functionality (but have just looked up that it was introduced as part of https://reviews.llvm.org/D100546). 

IIRC, based on the rationale given on https://reviews.llvm.org/D100546, the no-comdat functionality was needed for projects where the target environment does not support comdat functions. That suggests that for such projects, all functions should be marked to no-comdat.

Maybe it makes more sense to assert on when different nocomdat settings on different MachineFunctions would be observed rather than silently producing incorrect code (a comdat thunk, when a no-comdat thunk was requested?)

https://github.com/llvm/llvm-project/pull/97468


More information about the llvm-commits mailing list