This is an archive of the discontinued LLVM Phabricator instance.

CodeGen: Give pointer parameters 'align' attributes
Needs ReviewPublic

Authored by majnemer on Sep 17 2014, 9:27 PM.

Details

Summary

A pointer passed into a function must be suitably aligned or else risks
undefined behavior. Pass this information over to the middle end
optimizers by annotating pointer parameters with 'align' attributes.

Diff Detail

Event Timeline

majnemer updated this revision to Diff 13819.Sep 17 2014, 9:27 PM
majnemer retitled this revision from to CodeGen: Give pointer parameters 'align' attributes.
majnemer updated this object.
majnemer added reviewers: rsmith, hfinkel, nicholas, nlewycky.
majnemer added a subscriber: Unknown Object (MLST).
hfinkel added inline comments.Sep 18 2014, 2:33 PM
lib/CodeGen/CGCall.cpp
1592

I don't really know anything about ObjC, but just about all of the places in SemaDeclAttr that check for pointer types for relevant attributes check for:

Ty->isAnyPointerType() || Ty->isBlockPointerType()

So maybe you should check for block pointers too?

lib/Headers/avxintrin.h
788

Are the changes to this file related to the alignment change?

majnemer added inline comments.Sep 18 2014, 3:13 PM
lib/CodeGen/CGCall.cpp
1592

I don't know what the load alignment requirements are for block pointers. Omitting them is conservatively correct.

lib/Headers/avxintrin.h
788

Yes. These intrinsics are intended to be used with pointers with any alignment.

hfinkel added inline comments.Sep 18 2014, 3:29 PM
lib/CodeGen/CGCall.cpp
1592

Agreed. It is fine as-is unless someone who does know cares to chime in.

lib/Headers/avxintrin.h
788

Ah, good point. But that's a problem. I'd need to make the same change to lib/Headers/altivec.h, but I don't think I can because it will interfere with the overloading. Even here, we lose type checking ability.

I think we may need to add some kind of attribute that can specify these pointers as unaligned (which, coincidentally, is my http://reviews.llvm.org/D4635 currently under review), or we need to make these take pointers to an underaligned type:

typedef double unaligned_double __attribute__((aligned(1)));
_mm256_loadu_pd(unaligned_double const *__p)

Does that work? If not, maybe putting it in a transparent_union will work?

Finally, as this change points out, this has the possibility of breaking things. Should we add a flag, akin to -fno-strict-aliasing to disable this feature?

majnemer updated this revision to Diff 13907.Sep 21 2014, 3:27 AM
  • Address review comments
majnemer updated this revision to Diff 13908.Sep 21 2014, 3:39 AM
  • Don't tie relaxing pointer alignment to any sanitizers.
majnemer added inline comments.Sep 21 2014, 3:40 AM
lib/Headers/avxintrin.h
788

I've changed the intrinsics to take underaligned pointers to complete types and I've also added -fno-strict-pointer-alignment to disable this optimization.