[PATCH] D75661: Remove SequentialType from the type heirarchy.

Wed Mar 18 08:08:54 PDT 2020

nhaehnle added a comment.

The AMDGPU changes LGTM, modulo a stylistic nitpick.

As for the overall change, I'm perfectly fine with the general direction of SequentialType, though I don't know where we stand in terms of enough people having had a chance to look at it.

I'm concerned though about having getSequentialElementType and getSequentialNumElements apply to different sets of types. That's bound to lead to confusion which causes bugs. Can we have one name for all types that are homogenous sequences of the same element type, and a different name for all types which are *fixed-length* homogenous sequences?

================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUPromoteAlloca.cpp:370-372
+    if (VectorType::isValidElementType(ArrayTy->getElementType()) &&
+        ArrayTy->getNumElements() > 0)
+      VectorTy = arrayTypeToVecType(ArrayTy);
----------------
Please put braces for the outer if (multi-line body).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75661