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

[Sander de Smalen via Phabricator via llvm-commits]

sdesmalen added inline comments.


================
Comment at: llvm/lib/Transforms/IPO/GlobalOpt.cpp:1883
           return DT.dominates(S, L) &&
-                 DL.getTypeStoreSize(LTy) <= DL.getTypeStoreSize(STy);
+                 TypeSize::isKnownLE(DL.getTypeStoreSize(LTy),
+                                     DL.getTypeStoreSize(STy));
----------------
Globals are not allowed to be scalable, so this shouldn't use isKnownLE.


================
Comment at: llvm/lib/Transforms/Scalar/LoopIdiomRecognize.cpp:899
   // it allows better simplification of the +1.
-  if (DL->getTypeSizeInBits(BECount->getType()) <
-          DL->getTypeSizeInBits(IntPtr) &&
+  if (TypeSize::isKnownLT(DL->getTypeSizeInBits(BECount->getType()),
+                          DL->getTypeSizeInBits(IntPtr)) &&
----------------
Does this need to work for scalable types?


================
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()),
----------------
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89703