[PATCH] D80738: [llvm][SVE] IR intrinsic for LD1RO.

[Sander de Smalen via Phabricator via llvm-commits]

sdesmalen accepted this revision.
sdesmalen added a comment.
This revision is now accepted and ready to land.

Some nits, but LGTM otherwise.



================
Comment at: llvm/include/llvm/IR/IntrinsicsAArch64.td:1308
 def int_aarch64_sve_ld1   : AdvSIMD_1Vec_PredLoad_Intrinsic;
+def int_aarch64_sve_ld1ro : AdvSIMD_1Vec_PredLoad_Intrinsic;
 
----------------
super nit: move this under ld1rq?


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:1461
   case AArch64ISD::LD1S:              return "AArch64ISD::LD1S";
+  case AArch64ISD::LD1RO:             return "AArch64ISD::LD1RO";
   case AArch64ISD::LDNF1:             return "AArch64ISD::LDNF1";
----------------
super nit: same suggestions as above.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.h:251
   LD1S,
+  LD1RO,
   LDNF1,
----------------
super nit: move under ld1rq.


================
Comment at: llvm/test/CodeGen/AArch64/sve-intrinsics-ld1ro.ll:4
+;
+; LD1ROB
+;
----------------
perhaps somewhat unrelated, but I see that llvm/test/CodeGen/AArch64/sve-intrinsics-loads.ll contains the ld1rq tests. (Oddly enough that file also contains tests for ldnt1, but nothing else)
Maybe we can merge the ld1r* tests into one file?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D80738/new/

https://reviews.llvm.org/D80738