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
Locked
