This is an archive of the discontinued LLVM Phabricator instance.

[AArch64] Emit TBAA metadata for SVE load/store intrinsics
ClosedPublic

Authored by sdesmalen on Feb 9 2022, 12:09 AM.

Details

Summary

In Clang we can attach TBAA metadata based on the load/store intrinsics
based on the operation's element type.

This also contains changes to InstCombine where the AArch64-specific
intrinsics are transformed into generic LLVM load/store operations,
to ensure that all metadata is transferred to the new instruction.

There will be some further work after this patch to also emit TBAA
metadata for SVE's gather/scatter- and struct load/store intrinsics.

Diff Detail

Event Timeline

sdesmalen created this revision.Feb 9 2022, 12:09 AM
sdesmalen requested review of this revision.Feb 9 2022, 12:09 AM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptFeb 9 2022, 12:09 AM
paulwalker-arm accepted this revision.Feb 10 2022, 3:58 AM

I cannot say I fully understand all the connotations of this change but my gut feeling is that if failures occur it's likely the input program is malformed. Either way, we're early in the LLVM 15 development cycle so have plenty of time to react if necessary.

llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp
888–889

If I'm nit picking you use auto here but then choose the explicit return type (i.e. CallInst) for the MaskedStore local. Is there a reason for this or just muscle memory?

This revision is now accepted and ready to land.Feb 10 2022, 3:58 AM
sdesmalen added inline comments.Feb 11 2022, 12:18 AM
llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp
888–889

Is there a reason for this or just muscle memory?

I initially assumed the IRBuilder's result was a Value* because that's what most of its methods return, so wrote auto *Store = cast<Storeinst>(Builder.CreateStore(...)). When I realised it wasn't required, I remove the cast, but not the auto. I changed the code for LD1 after fixing ST1, hence the inconsistency.