[PATCH] D38313: [InstCombine] Introducing Aggressive Instruction Combine pass

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: lib/Transforms/AggressiveInstCombine/TruncInstCombine.cpp:205
+  if (MinBitWidth > TruncBitWidth) {
+    // Use the smallest integer type in the range [MinBitWidth, OrigBitWidth).
+    Type *Ty = DL.getSmallestLegalIntType(DstTy->getContext(), MinBitWidth);
----------------
I'm not sure you've addressed my vector concerns. The first part of this 'if' would create a new type for vectors by using getSmallestLegalIntType.


================
Comment at: lib/Transforms/AggressiveInstCombine/TruncInstCombine.cpp:217
+    bool ToLegal = MinBitWidth == 1 || DL.isLegalInteger(MinBitWidth);
+    if (FromLegal && !ToLegal)
+      return OrigBitWidth;
----------------
I think the !Vector check that was here was correct previously. We don't do isLegalnteger checks on the scalar types of vectors. For vectors we assume that if the type was present in the IR, the transform is fine. In this block, TruncWidth == MinBitWidth so the type existed in the original IR. My vector concerns were about the block above where we create new a new type.


https://reviews.llvm.org/D38313