[PATCH] D15525: [GCC] Attribute ifunc support in llvm

Rafael Ávila de Espíndola via llvm-commits llvm-commits at lists.llvm.org
Thu Dec 24 07:20:16 PST 2015


rafael added inline comments.

================
Comment at: include/llvm-c/Core.h:1890
@@ +1889,3 @@
+ */
+LLVMValueRef LLVMAddIFunc(LLVMModuleRef M, LLVMTypeRef Ty,
+                          LLVMValueRef Resolver, const char *Name);
----------------
I am not sure about adding a C api right away. Maybe wait a bit once the patch is in?

================
Comment at: include/llvm/IR/GlobalIndirectSymbol.h:63
@@ +62,3 @@
+  }
+  GlobalObject *getBaseObject(const DataLayout &DL, APInt &Offset) {
+    return dyn_cast<GlobalObject>(
----------------
Is this meaningful for ifunc?

IMHO it is a design mistake for GlobalAlias to take an arbitrary expression. If ifunc can take just a GlobalValue that would be nice.


================
Comment at: include/llvm/IR/Module.h:591
@@ +590,3 @@
+/// @name IFunc Iteration
+/// @{
+
----------------
Is there real value in these @{}?

================
Comment at: include/llvm/IR/Module.h:593
@@ +592,3 @@
+
+  ifunc_iterator       ifunc_begin()            { return IFuncList.begin(); }
+  const_ifunc_iterator ifunc_begin() const      { return IFuncList.begin(); }
----------------
clang-format

================
Comment at: lib/AsmParser/LLParser.cpp:719
@@ -710,3 +718,3 @@
 
-  if (Ty != PTy->getElementType())
+  if (IsAlias && Ty != PTy->getElementType())
     return Error(
----------------
Any type restrictions when it is not an alias?

================
Comment at: lib/AsmParser/LLParser.h:274
@@ -273,4 +273,3 @@
                      GlobalVariable::ThreadLocalMode TLM, bool UnnamedAddr);
-    bool ParseAlias(const std::string &Name, LocTy Loc, unsigned Linkage,
-                    unsigned Visibility, unsigned DLLStorageClass,
-                    GlobalVariable::ThreadLocalMode TLM, bool UnnamedAddr);
+    bool ParseIndirectSymbol(const std::string &Name, LocTy Loc,
+                             unsigned Linkage, unsigned Visibility,
----------------
Start functions with lowercase.

================
Comment at: lib/Bitcode/Writer/BitcodeWriter.cpp:781
@@ +780,3 @@
+  for (const GlobalIFunc &I : M->ifuncs()) {
+    // IFUNC: [ifunc type, resolver val#, linkage, visibility]
+    Vals.push_back(VE.getTypeID(I.getValueType()));
----------------
You are missing the address space in the comment.


http://reviews.llvm.org/D15525





More information about the llvm-commits mailing list