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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
