[PATCH] D92715: [Clang][RISCV] Define RISC-V V builtin types

[Craig Topper via Phabricator via cfe-commits]

craig.topper added inline comments.


================
Comment at: clang/include/clang/Basic/RISCVVTypes.def:32
+// - ElBits is the size of one element in bits (SEW).
+//
+// - IsSigned is true for vectors of signed integer elements and
----------------
NF argument isn't documented. And is always 1.


================
Comment at: clang/include/clang/Basic/TargetInfo.h:221
 
+  unsigned HasRISCVVTypes : 1;
+
----------------
This needs to be initialized to false in TargetInfo::TargetInfo() in TargetInfo.cpp I think. And then set to true from lib/Basic/Targets/RISCV.cpp


================
Comment at: clang/lib/AST/ASTContext.cpp:2178
+      Width = 0; \
+      Align = 128; \
+      break;
----------------
Does this alignment need to be this high? The VMV0 register class in the backend has an alignment of 64 for spills. Just wondering why they aren't consistent.


================
Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:779
+
+      uint64_t Size = 0; // This is a scalable vector. See DwarfUnit.cpp:1414.
+      auto Align = getTypeAlignIfRequired(BT, CGM.getContext());
----------------
Is there a function name in DwarfUnt.cpp that would be a better reference here? Line numbers aren't stable.


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

https://reviews.llvm.org/D92715