This is an archive of the discontinued LLVM Phabricator instance.

[AIX][PGO] Teach profile runtime to read build-id
ClosedPublic

Authored by w2yehia on Mar 27 2023, 8:44 AM.

Details

Summary

On AIX, the build-id can be embedded in a binary using the -mxcoff-build-id=0xHEXSTRING compiler option.
The build-id, if present, is stored as an ascii string at the beginning of the string table in the loader section of the XCOFF file.

Diff Detail

Event Timeline

w2yehia created this revision.Mar 27 2023, 8:44 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 27 2023, 8:44 AM
Herald added subscribers: Enna1, wenlei. · View Herald Transcript
w2yehia requested review of this revision.Mar 27 2023, 8:44 AM
compiler-rt/lib/profile/InstrProfilingPlatformAIX.c
24

This check is unnecessary. 'n' is the length of the input. The caller allocates an output buffer that's big enough, so overflow can't occur.

29

Since this function is only used when debugging, I don't see any reason to print 4 bytes at a time.

62

The usual convention for using loadquery is to keep allocating a buffer until the call succeeds or malloc() fails. But If you're going to limit the buffer size to 64K, then why not allocate a 64K buffer right away when you call malloc()? Of remove the fixed size Buf[1024] and always call malloc.

Also, I would use an integer (64000) instead of 1<<15.

compiler-rt/lib/profile/InstrProfilingWriter.c
347

BinaryIDPadding could be computed inside this function instead of passing it as a parameter.

w2yehia updated this revision to Diff 509103.Mar 28 2023, 12:43 PM
w2yehia marked 2 inline comments as done.
w2yehia added inline comments.
compiler-rt/lib/profile/InstrProfilingPlatformAIX.c
62

I'll change to do one malloc of 64k.
I think, it's still a good idea to avoid a by-default malloc. But this code runs once and when the program is shutting down, so maybe it's not a big deal? I'll defer to you on what's better.

compiler-rt/lib/profile/InstrProfilingWriter.c
347

This was an existing function so I wanted to do minimal changes.
As it is now, the writer function is logic-free, and it's upto the caller to decide the size of the padding.
Also, the padding size is used after calling lprofWriteOneBinaryId so it's good to compute it once and not to have to worry about keeping the two computations equivalent.

w2yehia marked an inline comment as done.Mar 28 2023, 12:48 PM
This revision is now accepted and ready to land.Mar 28 2023, 1:52 PM
w2yehia marked an inline comment as done.Mar 29 2023, 7:34 AM
w2yehia updated this revision to Diff 509366.Mar 29 2023, 7:50 AM
w2yehia retitled this revision from [AIX][PGO] Teach PGO rt to read build-id to [AIX][PGO] Teach profile runtime to read build-id.
w2yehia edited the summary of this revision. (Show Details)
This revision was landed with ongoing or failed builds.Mar 29 2023, 8:17 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptMar 29 2023, 8:17 AM
Herald added a subscriber: Restricted Project. · View Herald Transcript