[PATCH] D91250: Support intrinsic overloading on unnamed types

[Jeroen Dobbelaere via Phabricator via llvm-commits]

jeroen.dobbelaere added a comment.

I am doing a testrun with the assert. Based on that outcome I'll update the patches.



================
Comment at: llvm/docs/LangRef.rst:20074
 
-'``llvm.ssa_copy``' Intrinsic
+'``llvm.ssa.copy``' Intrinsic
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
----------------
fhahn wrote:
> submit the `llvm.ssa.copy` fixes separately?
ok. I can split it out.


================
Comment at: llvm/include/llvm/IR/Module.h:202
+      CurrentIntrinsicIds; ///< Keep track of the current unique id count for
+                           ///< the specified intrinsic basename
+  DenseMap<std::pair<Intrinsic::ID, const FunctionType *>, unsigned>
----------------
fhahn wrote:
> nit: period at end.
> nit: period at end.

What is the preference ? the comments before are also mixed wrt to the period.


================
Comment at: llvm/include/llvm/IR/Module.h:204
+  DenseMap<std::pair<Intrinsic::ID, const FunctionType *>, unsigned>
+      UniquedIntrinsicNames; ///< Keep track of uniqued names of intrinsics
+                             ///< based on unnamed types
----------------
fhahn wrote:
> Where those the name come in here? It maps intrinsic ids to other ids?
> Where those the name come in here? It maps intrinsic ids to other ids?

This map, based on the intrinsic ID and the FunctionType, allow us to remember what extension number has been associated to that combination. The uniqued name is based on the intrinsic ID, the FunctionType and that extension number.



================
Comment at: llvm/lib/IR/Function.cpp:809
+std::string Intrinsic::getName(ID id, ArrayRef<Type *> Tys) {
+  return getName(id, Tys, nullptr, nullptr);
+}
----------------
fhahn wrote:
> can we assert that there are no unnamed types involved here?
Should be possible. I would add the assert at line 796-797.


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

https://reviews.llvm.org/D91250