[PATCH] D76238: [SveEmitter] Implement builtins for contiguous loads/stores

[Sjoerd Meijer via Phabricator via cfe-commits]

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

Looks good to me. Please wait a day in case Eli has more comments.



================
Comment at: clang/include/clang/Basic/arm_sve.td:128
 def SVLD1   : MInst<"svld1[_{2}]", "dPc", "csilUcUsUiUlhfd", [IsLoad]>;
+def SVLD1SB : MInst<"svld1sb_{d}", "dPS", "silUsUiUl",       [IsLoad],              MemEltTyInt8>;
+def SVLD1UB : MInst<"svld1ub_{d}", "dPW", "silUsUiUl",       [IsLoad, IsZxtReturn], MemEltTyInt8>;
----------------
sdesmalen wrote:
> SjoerdMeijer wrote:
> > nit: don't think we have a coding style for tablegen, but it is exceeding 80 characters, even making this on phabricator a bit difficult to read, perhaps you can reshuffle this a bit.
> I gave it a try to avoid these long lines, but found that this made the .td file a lot less readable. If you don't have a strong opinion on this, I'd prefer to keep it as-is.
Ok, thanks for trying.


================
Comment at: clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_ld1sh.c:5
+//
+// ld1sh
+//
----------------
Nit: I really don't mind this, but this comment in its current form it doesn't add much (might as well remove it, or add a description, same for the other files)


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

https://reviews.llvm.org/D76238