[PATCH] D75470: [SVE] Auto-generate builtins and header for svld1.

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer added inline comments.


================
Comment at: clang/include/clang/Basic/arm_sve.td:121
+// Load one vector (scalar base)
+def SVLD1   : MInst<"svld1[_{2}]", "dPc", "csilUcUsUiUlhfd", [IsLoad]>;
----------------
sdesmalen wrote:
> SjoerdMeijer wrote:
> > This encoding, e.g, this is  "csilUcUsUiUlhfd", is such a monstrosity. It's a very efficient encoding, but of course completely unreadable.  I know there is prior art, and know that this is how it's been done, but just curious if you have given it thoughts how to do this in a normal way, a bit more c++y.  I don't want to de-rail this work, but if we are adding a new emitter, perhaps now is the time to give it a thought, so was just curious.  
> Haha, its a bit of a monstrosity indeed. The only thing I can think of here would be having something like:
> ```class TypeSpecs<list<string> val> {
>   list<string> v = val;
> }
> def All_Int_Float_Ty : TypeSpecs<["c", "s", "i", "l", "Uc", "Us", "Ul", "h", "f", "d">;
> 
> def SVLD1 : Minst<"svld1[_{2}]", "dPc", All_Int_Float_Ty, [IsLoad]>;```
> 
> But I suspect this gets a bit awkward because of the many permutations, I count more than 40. Not sure if that would really improve the readability.
I would personally welcome any improvement here, even the smallest. But if you think it's tricky, then fair enough!

I've managed to completely ignore the MVE intrinsics work so far, but understood there were some innovations here and there (e.g. in tablegen). Probably because it is dealing with similar problems: a lot of intrinsics, some of them overloaded with different types. I'm going to have a little look now to see if there's anything we can borrow from that, or if that is unrelated....


================
Comment at: clang/utils/TableGen/SveEmitter.cpp:64
+        Predicate(false), PredicatePattern(false), PrefetchOp(false),
+        Bitwidth(128), ElementBitwidth(~0U), NumVectors(1) {
+    if (!TS.empty())
----------------
sdesmalen wrote:
> SjoerdMeijer wrote:
> > why a default of 128? Will this gives problems for SVE implementions with> 128 bits?
> SVE vectors are n x 128bits, so the 128 is scalable here.
ah, okay, fair enough, didn't realise that.


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

https://reviews.llvm.org/D75470