[PATCH] D49434: Put "built-in" function definitions in global Used list, for LTO. (fix bug 34169)

[Peter Collingbourne via Phabricator via llvm-commits]

pcc added inline comments.


================
Comment at: include/llvm/CodeGen/GlobalISel/LegalizerHelper.h:28
 #include "llvm/CodeGen/MachineFunctionPass.h"
-#include "llvm/CodeGen/RuntimeLibcalls.h"
+#include "llvm/IR/RuntimeLibcalls.h"
 
----------------
I don't think we should move `RuntimeLibcalls.h` (because it references CodeGen internals), just `RuntimeLibcalls.def`.


================
Comment at: lib/Object/IRSymtab.cpp:26
 #include "llvm/IR/Module.h"
+#include "llvm/IR/RuntimeLibcalls.h"
 #include "llvm/Bitcode/BitcodeReader.h"
----------------
With my suggestion for the array you shouldn't need to include this header.


================
Comment at: lib/Object/IRSymtab.cpp:46
 
+static const char *LibcallRoutineNames[RTLIB::UNKNOWN_LIBCALL + 1];
+
----------------
I think this can just be a statically initialized array. You can use something like:
```
static const char *LibcallRoutineNames[] = {
#define HANDLE_LIBCALL(code, name) name,
#include "llvm/IR/RuntimeLibcalls.def"
};
```


================
Comment at: lib/Object/IRSymtab.cpp:239
 
-  if (Used.count(GV))
+  bool builtin_func = false;
+
----------------
Use CamelCase to match the surrounding style.


================
Comment at: lib/Object/IRSymtab.cpp:241
+
+  for (int i = 0; i < RTLIB::UNKNOWN_LIBCALL; ++i) {
+    const char *libcall_name = LibcallRoutineNames[(RTLIB::Libcall)i];
----------------
Use a range for loop to enumerate this array.


https://reviews.llvm.org/D49434