This is an archive of the discontinued LLVM Phabricator instance.

[SveEmitter] Add builtins for contiguous prefetches
ClosedPublic

Authored by sdesmalen on Apr 22 2020, 2:25 PM.

Details

Summary

This patch also adds the enum sv_prfop for the prefetch operation specifier
and checks to ensure the passed enum values are valid.

Diff Detail

Event Timeline

sdesmalen created this revision.Apr 22 2020, 2:25 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 22 2020, 2:25 PM
Herald added a subscriber: tschuett. · View Herald Transcript
sdesmalen updated this revision to Diff 259403.Apr 22 2020, 2:40 PM
  • Removed unrelated whitespace changes.
efriedma accepted this revision.Apr 22 2020, 3:01 PM

LGTM with a couple minor comments.

clang/lib/CodeGen/CGBuiltin.cpp
7722

It seems sort of silly to emit a no-op bitcast+gep+bitcast in the Ops.size() <= 3 case, but I guess it doesn't matter much.

clang/test/CodeGen/aarch64-sve-intrinsics/negative/acle_sve_prfb.c
23

Maybe worth adding a negative test for svprfb_vnum?

This revision is now accepted and ready to land.Apr 22 2020, 3:01 PM
sdesmalen updated this revision to Diff 259587.Apr 23 2020, 8:55 AM
sdesmalen marked an inline comment as done.
  • Don't emit bitcast+gep+bitcast when offset is 0.
  • Added negative tests for _vnum cases.
sdesmalen marked an inline comment as done.Apr 23 2020, 8:56 AM
sdesmalen added inline comments.
clang/lib/CodeGen/CGBuiltin.cpp
7722

You're right that was a bit silly. I've changed it now.

clang/test/CodeGen/aarch64-sve-intrinsics/negative/acle_sve_prfb.c
23

Yes good catch I've added them. I'm not necessarily sure we also need to add them for all the gather variants as well. I think just having tests where the position of the prefetch specifier is different should be sufficient.

efriedma accepted this revision.Apr 23 2020, 2:00 PM

LGTM

clang/test/CodeGen/aarch64-sve-intrinsics/negative/acle_sve_prfb.c
23

Yes, that seems fine.

This revision was automatically updated to reflect the committed changes.