ctetreau added inline comments.
================ Comment at: clang/lib/CodeGen/CGDecl.cpp:1078 } + // FIXME: Do we need to handle tail padding in vectors? return constant; ---------------- efriedma wrote: > 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. At a cursory glance, it seemed to me that the addition of this comment implied that it used to handle the tail padding. However, after having dug into what this function does, assuming that vectors only ever actually contain scalars, then it seems that this does currently just return the original vector and that this patch does not change the behavior. Given this, your comment really kind of just says "FIXME: is this function correct?", a question that applies to basically every function. If you have reason to believe that the answer to your question is yes, you should probably add why you think so to this comment. Otherwise, I think you should remove it. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75661/new/ https://reviews.llvm.org/D75661 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
