[cfe-dev] [RFC] Sort out and correct pragma align/pack stack effect

Xiangling Liao via cfe-dev cfe-dev at lists.llvm.org
Mon Dec 7 06:16:53 PST 2020


ping.

On Tue, Dec 1, 2020 at 10:16 AM Xiangling Liao <xiangxdh at gmail.com> wrote:

> ping.
>
> On Fri, Nov 27, 2020 at 3:09 PM Xiangling Liao <xiangxdh at gmail.com> wrote:
>
>> Hi,
>>
>>
>>
>> Recently we are trying to implement AIX pragma align/pack stack effect,
>> but noticed current implementation model in Clang has some limitations.
>>
>>
>>
>> That results from mixing `#pragma pack` and `#pragma align` by using only an
>> unsigned number to represent both so that they could override each other.
>>
>>
>>
>> Some examples we have been aware of are:
>>
>>
>>
>> 1. The current diagnostic for pragma pack does not produce a valid
>> diagnostic when align pragma is involved. For example,
>>
>>
>>
>> *$ cat test_diag.c*
>>
>> *#pragma align = natural *
>>
>>
>>
>> *$ clang test_diag.c -S *
>>
>>
>>
>> *test_diag.c:1:9: warning: unterminated '#pragma pack (push, ...)' at end
>> of file [-Wpragma-pack]*
>>
>> *#pragma align = natural *
>>
>> *        ^*
>>
>> *test_diag.c:1:9: note: did you intend to use '#pragma pack (pop)'
>> instead of '#pragma pack()'?*
>>
>> *1 warning generated.*
>>
>>
>>
>> The diagnostic is misleading by indicating some `*'#pragma pack *` is
>> unterminated. This is caused by not differentiating between align pragma
>> and pack pragma in clang’s current align and pack stack model.
>>
>>
>>
>>
>>
>> 2*.*There exists platforms like AIX that cannot have pragma align/pack
>> stack effect simply represented by an alignment value. The stack effect
>> between align and pack pragmas on AIX is different.
>>
>>
>>
>> (1)They don’t override but create combinations with each other sometimes.
>>
>>
>>
>> Eg.
>>
>> #pragma align(natural)
>>
>> #pragma pack(2)
>>
>>
>>
>> In current Clang’s implementation we get pack(2) in effect here without
>> knowing align mode information above it;
>>
>>
>>
>> However, XL on AIX would need to align objects to natural alignment first and then reduced by pack(2).
>>
>>
>>
>> (2)Additional complication comes in with `#pragma align(reset)` and `#pragma pack(pop)`.
>>
>>
>>
>> Eg.
>>
>> #pragma pack(2)
>>
>> #pragma align = packed
>>
>> #pragma pack(pop)
>>
>>
>>
>> In current Clang’s implementation we get pack(2) in effect here.
>>
>>
>>
>> However, for XL on AIX, `#pragma pack(pop)` won’t be able to pop the stack, because `#pragma align` sets a baseline, and there is no pragma pack applied to it. Without distinguishing pragma align and pramgma pack, the `#pragma align = packed` would be popped out mistakenly for AIX.
>>
>>
>>
>> *3.* Features added in later like PCH only record pragma alignment
>> number. In platforms like AIX, baseline align mode information will be lost.
>>
>>
>>
>>
>> Based on current limitations, we are proposing a new model by splitting
>> up the alignment value into an align mode and a pack number so that we
>> would be able to know what is the combined effect of align and pack in our
>> stack.
>>
>>
>>
>> The proposed patch has been posted here as a reference:
>>
>> https://reviews.llvm.org/D87702
>>
>>
>>
>> This patch implements the proposed new model,  implements AIX pragma
>> align/stack effect based on that, and updates pragma align/pack related PCH
>> handling process.
>>
>>
>>
>> But there are several things I would like to point out particularly:
>>
>>
>>
>> (1) In `clang/include/clang/Sema/Sema.h`,
>>
>>     bool operator==(const AlignPackInfo &Info) const {
>>
>>       return std::tie(AlignMode, PackNumber, PackAttr, AIXStack) ==
>>
>>              std::tie(Info.AlignMode, Info.PackNumber, Info.PackAttr,
>>
>>                       Info.AIXStack);
>>
>> }
>>
>>
>>
>> We are trying to pursue a stricter “equal” definition of if two pragma
>> are equal. That would change some previous behavior, for example:
>>
>>
>>
>> *a) *
>>
>> *#pragma align(packed)*
>>
>> *#pragma pack(1) // AlignMode = Packed,PackNumber = 1,IsPackAttr*
>>
>>
>>
>> *b)*
>>
>> *#pragma align(packed) //AlignMode = Packed,PackNumber = 1,  *
>>
>> *                        IsAlignAttr*
>>
>>
>>
>>
>>
>> Since we only care about unsigned numbers previously, these two pragma
>> settings would be treated as equal.
>>
>>
>>
>> But now, they would not. So on platforms where the previous result is
>> required, we would need to compare the `PackNumber` of `AlignPackInfo`
>> objects instead of comparing themselves directly.
>>
>>
>>
>> (2) In the first diagnostic limitation mentioned above, we handle it by
>> leaving a FIXME in the current patch in order to avoid scope creep. But we
>> believe the new `AlignPackInfo` struct provides a good foundation to
>> address this issue in the future.
>>
>>
>>
>> Please let me know if there are any questions about our current proposal
>> and design. Your feedback is appreciated.
>>
>>
>>
>> Regards,
>>
>> Xiangling Liao
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20201207/c110c15a/attachment.html>


More information about the cfe-dev mailing list