[PATCH] D88086: [ARM][MVE] tail-predication: checks for the elementcount, cont'd

[Eli Friedman via Phabricator via llvm-commits]

efriedma added inline comments.


================
Comment at: llvm/lib/Target/ARM/MVETailPredication.cpp:405
     // ii) TC1 has to be equal to TC + 1, with the + 1 to compensate for start
     //     counting from 0.
     uint64_t TC2 = ConstElemCount->getZExtValue() + 1;
----------------
Maybe add a comment clarifying why we want to bail out here?


================
Comment at: llvm/lib/Target/ARM/MVETailPredication.cpp:433
   auto MaxMinusVW = APInt(SizeInBits, ~0) - APInt(SizeInBits, VectorWidth);
   APInt UpperboundTC = SE->getUnsignedRangeMax(TC);
 
----------------
Should this be using EC, not TC?


================
Comment at: llvm/lib/Target/ARM/MVETailPredication.cpp:480
   }
   if (!RangeTC.contains(RangeCeil) && !ForceTailPredication) {
     LLVM_DEBUG(dbgs() << "ARM TP: Overflow possible in sub\n");
----------------
I'm not confident this is actually proving what you want to prove?  If x is in the range [0, 10), and y is in the range [0, 12), that doesn't prove x<y.


================
Comment at: llvm/lib/Target/ARM/MVETailPredication.cpp:507
     return false;
   }
   auto StepValue = Step->getValue()->getSExtValue();
----------------
Do we need to check the first operand of the AddRec is zero?


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

https://reviews.llvm.org/D88086