[PATCH] D66871: [SVE] MVT scalable size queries
Diana Picus via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Nov 13 03:47:52 PST 2019
rovka added a comment.
Regarding the change to return const, I'm not convinced that's a good idea (we actually have a clang-tidy check <https://clang.llvm.org/extra/clang-tidy/checks/readability-const-return-type.html> that warns about that). I think it would be better to either name those temporaries or use std::make_tuple instead of std::tie (whichever you prefer).
================
Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:13979
+ Offset = ((int64_t)STMemType.getStoreSizeInBits() -
+ (int64_t)LDMemType.getStoreSizeInBits()) / 8 - Offset;
----------------
Could you be a bit more specific about why you need to cast to signed? It was unsigned in the original code too.
================
Comment at: llvm/utils/TableGen/CodeGenDAGPatterns.cpp:515
+ return std::tie(A.getScalarSizeInBits(), A.getSizeInBits()) <
+ std::tie(B.getScalarSizeInBits(), B.getSizeInBits());
};
----------------
I was actually thinking of including isScalableVector and isVector below in the tie (since bool is an integral type and compares the way you'd expect).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D66871/new/
https://reviews.llvm.org/D66871
More information about the llvm-commits
mailing list