SjoerdMeijer added a comment.
Big patch, I only did a first scan, but looks very reasonable in general. Just
a first round of nits and 2 questions.
================
Comment at: clang/include/clang/Basic/arm_sve.td:121
+// Load one vector (scalar base)
+def SVLD1 : MInst<"svld1[_{2}]", "dPc", "csilUcUsUiUlhfd", [IsLoad]>;
----------------
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.
================
Comment at: clang/utils/TableGen/SveEmitter.cpp:1
//===- SveEmitter.cpp - Generate arm_sve.h for use with clang -*- C++ -*-===//
//
----------------
I wanted to add the nit that SveEmiiter.cpp should perhaps be SVEEmitter.cpp,
but then I saw at the bottom that MVE is spelled Mve, so perhaps this is fine
then.
================
Comment at: clang/utils/TableGen/SveEmitter.cpp:64
+ Predicate(false), PredicatePattern(false), PrefetchOp(false),
+ Bitwidth(128), ElementBitwidth(~0U), NumVectors(1) {
+ if (!TS.empty())
----------------
why a default of 128? Will this gives problems for SVE implementions with> 128
bits?
================
Comment at: clang/utils/TableGen/SveEmitter.cpp:119
+class Intrinsic {
+ /// The Record this intrinsic was created from.
+ Record *R;
----------------
nit: for readability, perhaps don't abbreviate some of these member names?
R -> Record
BaseTS -> BaseTypeSpec
CK -> ClassKind
================
Comment at: clang/utils/TableGen/SveEmitter.cpp:263
+unsigned SVEType::getTypeFlags() const {
+ unsigned Base = 0;
+
----------------
don't need this
================
Comment at: clang/utils/TableGen/SveEmitter.cpp:267
+ switch (ElementBitwidth) {
+ case 16: Base = (unsigned)SVETypeFlags::Float16; break;
+ case 32: Base = (unsigned)SVETypeFlags::Float32; break;
----------------
just a return here
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D75470/new/
https://reviews.llvm.org/D75470
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
