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

sdesmalen added inline comments. Comment at: clang/include/clang/Basic/arm_sve.td:186 +def SVLDFF1 : MInst<"svldff1[_{2}]", "dPc", "csilUcUsUiUlhfd", [IsLoad], MemEltTyDefault, "aarch64_sve_ldff1">; +def SVLDFF1SB : MInst<"svldff1sb_{d}", "dPS", "silUsUiUl",

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

Andrzej added inline comments. Comment at: clang/include/clang/Basic/arm_sve.td:186 +def SVLDFF1 : MInst<"svldff1[_{2}]", "dPc", "csilUcUsUiUlhfd", [IsLoad], MemEltTyDefault, "aarch64_sve_ldff1">; +def SVLDFF1SB : MInst<"svldff1sb_{d}", "dPS", "silUsUiUl",

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

sdesmalen added a comment. In D76238#1954585 , @SjoerdMeijer wrote: > Thanks for the ping, hadn't noticed the updates. > > I may have also missed why you need the SVE_ACLE_FUNC macro. Can you quickly > comment why you need that? Just asking because in

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

SjoerdMeijer added a comment. Thanks for the ping, hadn't noticed the updates. I may have also missed why you need the SVE_ACLE_FUNC macro. Can you quickly comment why you need that? Just asking because in my opinion it doesn't really make it more pleasant to read (so it's there for another

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

sdesmalen added a comment. @SjoerdMeijer are you happy with the changes I've made to the tests? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76238/new/ https://reviews.llvm.org/D76238 ___ cfe-commits mailing list

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

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}]",

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

sdesmalen marked 4 inline comments as done. sdesmalen added inline 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],

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

efriedma added inline comments. Comment at: clang/include/clang/Basic/AArch64SVETypeFlags.h:72 + bool isStructStore() const { return Flags & IsStructStore; } + bool isZxtReturn() const { return Flags & IsZxtReturn; } + SjoerdMeijer wrote: > nit: this one is

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

SjoerdMeijer added inline comments. Comment at: clang/lib/CodeGen/CGBuiltin.cpp:7467 + + return IsZxtReturn ? Builder.CreateZExt(Load, VectorTy) + : Builder.CreateSExt(Load, VectorTy); sdesmalen wrote: > SjoerdMeijer wrote: > > nit: and now

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

sdesmalen marked 2 inline comments as done. sdesmalen added inline comments. Comment at: clang/lib/CodeGen/CGBuiltin.cpp:7467 + + return IsZxtReturn ? Builder.CreateZExt(Load, VectorTy) + : Builder.CreateSExt(Load, VectorTy); SjoerdMeijer

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

SjoerdMeijer added a comment. Some nits inlined Comment at: clang/include/clang/Basic/AArch64SVETypeFlags.h:72 + bool isStructStore() const { return Flags & IsStructStore; } + bool isZxtReturn() const { return Flags & IsZxtReturn; } + nit: this one is non