ctetreau added a comment. > 1. This is orthogonal to splitting VectorType, as far as I can tell. > `Ty->getVectorNumElements()` works equally well whether the implementation > asserts it's a VectorType that isn't scalable, or asserts it's a > FixedVectorType.
While it's not strictly neccesary, the refactor will be more straightforward with this change. Before, we would have: unsigned n = t->getVectorNumElements(); After the VectorTypes refactor, if t is not a FixedLengthVector, then this code will still compile but fail at runtime. However with: unsigned n = cast<VectorType>(t)->getNumElements(); ... it will be a compile time failure. Realistically, it would need to be at least renamed to getFixedVectorNumElements anyways to avoid confusion. > 2. This needs to be split up; it's way too long to read, and it's mixing > functional and non-functional changes. I can split it up into a few chunks that make the corrections a bit at a time, then a final change to get rid of the functions > 3. It might make sense to add helpers to remove the use of cast<> in more > places. For example, the inputs of a shufflevector are guaranteed to be > vectors. It seems to me that we use base Type for basically everything when we could use concrete subclasses. As you can see in my patch, I fixed a few, but ideally functions would just return a VectorType if they have a VectorType. The issue is pervasive, and is enabled by the existence of these functions. This is a deep rabbit hole that I don't really want to go down in this patch. > 4. Some of the changes are leading to really crazy indentation because the > `cast<>` is more characters. While I agree, there's not much we can do about that. Really, the problem is that we have these crazy complicated if statements that check 15 things at once. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77236/new/ https://reviews.llvm.org/D77236 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
