This is an archive of the discontinued LLVM Phabricator instance.

[clang][CodeGenCXX] Improve handling of itanium ABI member function alignment requirements
ClosedPublic

Authored by daltenty on Mar 29 2023, 2:05 PM.

Details

Summary

The itanium ABI for certain platforms requires a minimum alignments for
member function pointers to reserve certain bits for distinguishing
virtual and non-virtual functions.

Clang's implementation of this however depends on the alignment of the
function involved, which may however not reflect the true alignment of
function pointers on certain targets for which the alignment is
independent of the function (e.g. AIX). Worse, the 2-byte alignment
we use may be less than the ABI minimum for the target, and in the case
we are using explicit sections will result in invalid codegen.

This patch attempts to correct this situation by considering the target
alignment of function pointers as part of making the decision about
whether we need to adjust the function alignment to conform to the ABI.
Targets which do not provide the function ptr alignment information
will return a value of 1 when queried and will conservatively retain
the old alignment.

Diff Detail

Event Timeline

daltenty created this revision.Mar 29 2023, 2:05 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 29 2023, 2:05 PM
daltenty requested review of this revision.Mar 29 2023, 2:05 PM
clang/lib/CodeGen/CodeGenModule.cpp
2152

Out-of-scope comment: Only non-static member functions need this.

2153

Is there a platform where the pointer alignment is less than 2 when the function has a higher alignment? If there is one, then let's test the effect of this line explicitly.

daltenty added inline comments.Jul 5 2023, 1:52 PM
clang/lib/CodeGen/CodeGenModule.cpp
2153

Is there a platform where the pointer alignment is less than 2 when the function has a higher alignment?

TL;DR: ARM is a platform like this. I've added an extra test in https://reviews.llvm.org/rGcf08c103266b7ee59cbc2352c4d20f27472cb257 to demonstrate nothing changes.

  1. Function ptr alignment follows function alignment
  2. Function ptr alignment follows function alignment; and: a) function ptr alignment > 2 b) function ptr alignment < 2

Previous to this patch, this code assumed 1. Afterwards, we handle 1 and 2a here.

For a platform which case 2b, which is what you describe, you can't use this mechanism for discriminating between virtual and non-virtual member functions, so such a platform should really return false for getCXXABI().areMemberFunctionsAligned(). But platforms in this case don't like ARM, which notes that TODO here: https://github.com/llvm/llvm-project/blob/cfbcbc8f88eda598a42806fdb92bef107d23c898/clang/include/clang/Basic/TargetCXXABI.h#L168. Those platforms will simply continue to observe the extra unneeded alignment (2) or the specified alignment, which ever is greater, as they did before.

daltenty updated this revision to Diff 537486.Jul 5 2023, 1:55 PM

Rebase on latest main

This revision is now accepted and ready to land.Jul 5 2023, 4:02 PM
This revision was landed with ongoing or failed builds.Jul 6 2023, 7:36 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptJul 6 2023, 7:36 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript