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

Eli Friedman via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Mar 20 10:50:30 PDT 2020


efriedma marked 2 inline comments as done.
efriedma added inline comments.


================
Comment at: clang/lib/CodeGen/CGDecl.cpp:1059
+    llvm::Type *ElemTy = ArrayTy->getElementType();
+    bool ZeroInitializer = constant->isNullValue();
     llvm::Constant *OpValue, *PaddedOp;
----------------
ctetreau wrote:
> I assume the reasoning here is that [-0.0f, ...] isn't the zero initialized value, but why make this change now? Seems unrelated.
I'll post this separately for review, since it's a little complicated.


================
Comment at: clang/lib/CodeGen/CGDecl.cpp:1078
   }
+  // FIXME: Do we need to handle tail padding in vectors?
   return constant;
----------------
ctetreau wrote:
> The fact that you have to ask this question tells me that you should probably just make this handle vectors.
> 
> You could add a templated helper function above this function that is basically just the original body of the SequentialType branch.
Well, no, the original code doesn't handle vectors either.  The issue here would be that we need to pad out the vector with additional elements, or something like that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75661





More information about the llvm-commits mailing list