[PATCH] D89703: [SVE]Clarify TypeSize comparisons in llvm/lib/Transforms

[Sander de Smalen via Phabricator via llvm-commits]

sdesmalen accepted this revision.
sdesmalen added a comment.
This revision is now accepted and ready to land.

LGTM!



================
Comment at: llvm/lib/Transforms/Scalar/NaryReassociate.cpp:378
   if (isKnownNonNegative(LHS, *DL, 0, AC, GEP, DT) &&
-      DL->getTypeSizeInBits(LHS->getType()) <
-          DL->getTypeSizeInBits(GEP->getOperand(I)->getType())) {
+      TypeSize::isKnownLT(
+          DL->getTypeSizeInBits(LHS->getType()),
----------------
CarolineConcatto wrote:
> sdesmalen wrote:
> > This is a functional change that now allows mixing scalable and fixed-width types. Either that should not happen (and thus it shouldn't use isKnownLT) or it should have a test that tests the new behaviour.
> No vector(fix-width or scalable) will go down this path 
> The test:
>  if (SE->isSCEVable(I->getType()) && isPotentiallyNaryReassociable(&*I))
> inside the method:
> bool NaryReassociatePass::doOneIteration(Function &F)
> prevents these types to reach this path.
Okay that makes sense, isSCEVable returns false on vector types. Thanks for confirming!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89703