Page MenuHomePhabricator

[Clang]Replace aarch64_sve_ldN intrinsic by aarch64_sve_ldN.sret
ClosedPublic

Authored by CarolineConcatto on Aug 11 2022, 8:42 AM.

Diff Detail

Event Timeline

CarolineConcatto requested review of this revision.Aug 11 2022, 8:42 AM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptAug 11 2022, 8:42 AM
Matt added a subscriber: Matt.Aug 11 2022, 12:00 PM
sdesmalen added inline comments.Aug 15 2022, 1:15 AM
llvm/test/CodeGen/AArch64/sve-calling-convention-mixed.ll
22 ↗(On Diff #451872)

There is something going wrong here, because it should not have been lowered to a call.

  • Use a correct intrinsic name for vector.insert. Before it was llvm.aarch64.vector.insert.
llvm/test/CodeGen/AArch64/sve-calling-convention-mixed.ll
22 ↗(On Diff #451872)

Thank you Sander.
Good spot. I left aarch64 in the intrinsic name.

sdesmalen added inline comments.Aug 15 2022, 3:32 AM
llvm/test/CodeGen/AArch64/sve-calling-convention-mixed.ll
127 ↗(On Diff #452619)

I would have expected no asm changes. The test is not equivalent to the previous code.

  • Fix sve-calling-convention-mixed.ll test
CarolineConcatto marked an inline comment as done.Aug 15 2022, 7:06 AM
CarolineConcatto added inline comments.
llvm/test/CodeGen/AArch64/sve-calling-convention-mixed.ll
127 ↗(On Diff #452619)

Yes, you were correct. I was taken the same load twice.

Can you also split this patch in two:

  • One for Clang where it will no longer use the legacy ld2/3/4 intrinsics
  • One for LLVM where it removes the old intrinsics and AutoUpgrades the old intrinsics.
CarolineConcatto marked an inline comment as done.
  • Removes llvm changes from this patch
CarolineConcatto retitled this revision from [AArch64] Replace aarch64_sve_ldN intrinsic by aarch64_sve_ldN.sret to [Clang]Replace aarch64_sve_ldN intrinsic by aarch64_sve_ldN.sret.Aug 19 2022, 1:06 AM
sdesmalen accepted this revision.Aug 19 2022, 1:13 AM

LGTM with nit addressed, thanks @CarolineConcatto!

clang/lib/CodeGen/CGBuiltin.cpp
8874

nit: s/VTy->getElementCount().getKnownMinValue()/VTy->getMinNumElements()/

This revision is now accepted and ready to land.Aug 19 2022, 1:13 AM