[PATCH] D105067: [SystemZ] Emit .gnu_attribute for an externally visible vector abi.

[Ulrich Weigand via Phabricator via llvm-commits]

uweigand added inline comments.


================
Comment at: clang/lib/CodeGen/TargetInfo.cpp:7393
+  mutable bool HasVisibleVecABIFlag = false;
+  mutable std::set<const Type *> SeenTypes;
+
----------------
I'm wondering if it wouldn't be preferable to move those (and the functions operating on them) to `SystemZTargetCodeGenInfo` instead?  There is precedent for `mutable` members in `XCodeTargetCodeGenInfo`.


================
Comment at: clang/lib/CodeGen/TargetInfo.cpp:7529
+    }
+  }
+  if (const auto *FT = Ty->getAs<FunctionType>())
----------------
I think this will also have to look into C++ base classes, similar to what is done in `isSingleElementStruct`, doesn't it?


================
Comment at: clang/lib/CodeGen/TargetInfo.cpp:7663
   CharUnits DirectAlign;
+  if (!CGF.CurFn->hasLocalLinkage())
+    handleExternallyVisibleObjABI(Ty.getTypePtr());
----------------
Not sure about this.   I think we need to do this unconditionally.  Think e.g. of a global function that gets called with a `va_list` argument, passes that `va_list` to another *local* function, which invokes the `va_arg` on it.  This would not be caught by your current code.


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

https://reviews.llvm.org/D105067