This is an archive of the discontinued LLVM Phabricator instance.

[AIX] Fix mcount name and call arguments
ClosedPublic

Authored by francii on Oct 6 2022, 11:28 AM.

Details

Summary

Currently, compiling a program with the -pg flag will result in an undefined symbol error for .mcount. This revision fixes the call to use __mcount, which requires a pointer argument to a pointer-sized object (unique per inserted call) on AIX.

This is only a partial fix. This patch should fix the -pg flag's behaviour on AIX to work with code you are compiling, but it will not link against standard libraries with mcount instrumentation calls. The next step is to add profiled libraries to the linker search paths in the Clang driver for the AIX toolchain when linking with -pg.

Diff Detail

Event Timeline

francii created this revision.Oct 6 2022, 11:28 AM
Herald added a project: Restricted Project. · View Herald TranscriptOct 6 2022, 11:28 AM
Herald added a subscriber: hiraditya. · View Herald Transcript
francii requested review of this revision.Oct 6 2022, 11:28 AM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptOct 6 2022, 11:28 AM
francii edited the summary of this revision. (Show Details)Oct 6 2022, 5:22 PM
cebowleratibm added inline comments.Oct 7 2022, 6:57 AM
clang/test/CodeGen/mcount-aix.c
3

Is it necessary to specify -no-opaque-pointers for this test?

hubert.reinterpretcast retitled this revision from Enable the use of the -pg flag to [AIX] Enable the use of the -pg flag.Oct 7 2022, 12:38 PM
hubert.reinterpretcast edited the summary of this revision. (Show Details)Oct 7 2022, 12:41 PM
hubert.reinterpretcast edited the summary of this revision. (Show Details)
cebowleratibm added inline comments.Oct 7 2022, 2:02 PM
clang/lib/Basic/Targets/OSTargets.h
754

Suggest sticking with the normal mcount name and remapping on the call insertion when the target is AIX.

francii updated this revision to Diff 466248.Oct 7 2022, 9:10 PM

Removed use of -no-opaque-pointers and call to "__mcount.aix"

francii updated this revision to Diff 466252.Oct 7 2022, 9:41 PM

Removed __mcount.aix usage

francii updated this revision to Diff 466254.Oct 7 2022, 10:03 PM

Replace target pointers with opaque pointers

cebowleratibm added inline comments.Oct 11 2022, 7:14 AM
llvm/lib/Transforms/Utils/EntryExitInstrumenter.cpp
39

Please use isOSAIX

francii updated this revision to Diff 466852.Oct 11 2022, 10:15 AM

Updated target triple check to use isOSAIX()

llvm/lib/Transforms/Utils/EntryExitInstrumenter.cpp
38–56

A Twine can be implicitly constructed from a std::string. The implicit conversion works in this context.
Also: Change the variable name to use the LLVM naming convention for non-"functions".

40

I see no reason why we should assume that all of the other function names should have the extra argument passed on AIX.

francii updated this revision to Diff 467351.Oct 12 2022, 8:16 PM

"Added check for __mcount on aix

LGTM with minor comment; thanks!

llvm/lib/Transforms/Utils/EntryExitInstrumenter.cpp
39

The construction for the Triple already did the string processing.

This revision is now accepted and ready to land.Oct 12 2022, 8:55 PM
francii updated this revision to Diff 467491.Oct 13 2022, 8:28 AM

Updated check for Func name

francii edited the summary of this revision. (Show Details)Oct 17 2022, 12:41 PM
francii retitled this revision from [AIX] Enable the use of the -pg flag to [AIX] Fix mcount name on AIX.Oct 17 2022, 12:44 PM
francii retitled this revision from [AIX] Fix mcount name on AIX to [AIX] Fix mcount name and arguments.
francii retitled this revision from [AIX] Fix mcount name and arguments to [AIX] Fix mcount name and call arguments.Oct 17 2022, 12:47 PM
francii edited the summary of this revision. (Show Details)
hubert.reinterpretcast edited the summary of this revision. (Show Details)Oct 17 2022, 12:51 PM
hubert.reinterpretcast edited the summary of this revision. (Show Details)
hubert.reinterpretcast edited the summary of this revision. (Show Details)Oct 17 2022, 12:53 PM
cebowleratibm closed this revision.Jan 10 2023, 7:45 AM

I had a typo in the commit so this didn't auto close.