[PATCH] D136192: [PGO][AIX] Improve dummy var retention and allow -bcdtors:csect linking.

wael yehia via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Oct 25 10:28:48 PDT 2022


w2yehia added a comment.

> I wasn't included as a reviewer on D124857 <https://reviews.llvm.org/D124857> and I missed that change so couldn't comment there, but I'm not a fan of including the AIX support in InstrProfilingPlatformLinux.c. AIX is neither Linux nor ELF-based and big chunks of that file are now #ifdefed out making it harder to comprehend which part is used where. I'd prefer introducing InstrProfilingPlatformAIX.c, moving the AIX-specific logic there, and then figuring out how to possibly share common parts between InstrProfilingPlatformLinux.c and InstrProfilingPlatformAIX.c, for example by moving them to an .inc file.

Splitting was considered but because AIX reuses the entirety of that file (except two `#include`'s) and adds some AIX-only stuff it seemed better not to duplicate the common code. Doing a .inc file was not considered. The `.inc` file will have to contain the entire `InstrProfilingPlatformLinux.c` content. 
Let me know if you want me to post a patch for the `.inc`. Thanks



================
Comment at: compiler-rt/lib/profile/InstrProfilingPlatformLinux.c:265-267
+COMPILER_RT_VISIBILITY
+void *__llvm_profile_keep[] = {(void *)&dummy_cnts, (void *)&dummy_data,
+                               (void *)&dummy_name, (void *)&dummy_vnds};
----------------
phosek wrote:
> Does AIX linker support the `retain` attribute? That would be a cleaner solution.
thanks for pointing out. Unfortunately the `retain` attribute is not supported by the AIX linker. It could be implemented on the compiler side, but as of now it's not supported by the build compiler on AIX. 


================
Comment at: compiler-rt/lib/profile/InstrProfilingRuntime.cpp:20-22
 COMPILER_RT_VISIBILITY int INSTR_PROF_PROFILE_RUNTIME_VAR;
-}
 
+static int Registration = RegisterRuntime();
----------------
phosek wrote:
> Is it possible to reuse the runtime hook variable on other platforms as well? I'd really like to avoid introducing another variable.
I also wanted to do that but didn't in fear of breaking others. I can do the change and see what breaks?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136192



More information about the cfe-commits mailing list