[PATCH] D118696: Fix incorrect TypeSize->uint64_t cast in InductionDescriptor::isInductionPHI

[Sander de Smalen via Phabricator via llvm-commits]

sdesmalen accepted this revision.
sdesmalen added a comment.

LGTM with comments addressed.



================
Comment at: llvm/lib/Analysis/IVDescriptors.cpp:1432
+  TypeSize TySize = DL.getTypeAllocSize(ElementType);
+  if (TySize.isZero() || TySize.isScalable())
     return false;
----------------
nit: I think this case //could// be supported, although that would require more work. Maybe you can leave a comment to say why we bail out for scalable vectors, and suggest there may be more work we can do here.


================
Comment at: llvm/test/Transforms/CanonicalizeFreezeInLoops/phis.ll:116
+
+; Negative test - ensure we handle scalable vector types correctly
+define void @no_freeze_scalable(<vscale x 4 x float>* %ptr) {
----------------
This test can probably be removed now? I don't think testing the code for this specific pass serves any specific purpose now that you've got the unittest.


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

https://reviews.llvm.org/D118696