Automagic strengthening of alignment of pointer parameters through a cast

I recently came across an issue that causes incorrect code gen in some cases on
PowerPC. Namely, when loading a different type from a pointer passed as a
parameter, the alignment emitted on the load will be the alignment of the
result type.
Case in point:
vector unsigned int test(unsigned int *a) {
return (vector unsigned int)a;
}
The natural alignment of vectors on PPC is 16-bytes. The natural alignment of
int is 4. When clang is emitting the IR for this, it’ll use the alignment of
the result of the cast on the load. So in this case, the alignment is unduly
strengthened which can produce incorrect code.

The following patch fixes the problem, but seems to be in direct contradiction
to the intent of the block of code it modifies.

Index: lib/CodeGen/CGExpr.cpp

I think the fix needs to be a little bit more subtle. The C standard states that it’s undefined behaviour for the result of a cast to be under aligned when derererenced, so in the general case if casting from X* to Y* and then dereferencing the result, the pointer must have the alignment of Y. In effect, when you cast from int to v4i, you are asserting to the compiler that the alignment is correct. To get the result that you want, you should be using the alignas attribute or similar on the destination cast.

That said, it looks as if clang always uses natural alignment when it means C standard (ABI-defined) alignment, which is a bug. There probably ought to be a hook to allow targets to weaken the alignment guarantees provided by a type.

Your patch, as is, would cause regressions on a lot of targets. Casting a char* to an int*, for example, would leave it with 1-byte alignment and if the target doesn’t support unaligned accesses for 4-byte values (e.g. some MIPS, many GPUs) then you’d get much slower code. Even on targets such as x86, for vectors you’d now be emitting the unaligned variants instead of the aligned ones.

David

Thank you David,

I think based on the statements in the standard, this is undefined behaviour. UBSan certainly seems to think so.