[PATCH] D29631: SystemZTargetTransformInfo cost functions and some common code changes

[Jonas Paulsson via Phabricator via llvm-commits]

jonpa added inline comments.


================
Comment at: include/llvm/CodeGen/BasicTTIImpl.h:315
     for (const Value *A : Args) {
-      if (UniqueOperands.insert(A).second)
-        Cost += getScalarizationOverhead(VectorType::get(A->getType(), VF),
-                                         false, true);
+      if (!isa<UndefValue>(A) && UniqueOperands.insert(A).second) {
+        Type *VecTy;
----------------
This part has been approved by Hal Finkel on llvm-commits already (just the first part here that handles scalar/vector argument types).




================
Comment at: lib/Target/SystemZ/SystemZTargetTransformInfo.h:30
 
+  unsigned const LIBCALL_COST = 30;
+
----------------
rengolin wrote:
> Shouldn't this just be the cost of a call, rather than being a magic constant?
Maybe, but that is also a magic constant of 10 :-)
Is there any point? This is used just for FRem.


================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:7236-7244
+    // If this is the loop-latch compare for the back branch, just add the
+    // scalar value. Should this check be done in caller instead?
+    bool LikelyVectorized = true;
+    if (I->hasOneUse()) {
+      if (BranchInst *BI = dyn_cast<BranchInst>(I->use_begin()->getUser())) {
+        if (BI->getParent() == TheLoop->getLoopLatch())
+          LikelyVectorized = false;
----------------
mssimpso wrote:
> mssimpso wrote:
> > Hi,
> > 
> > I've only looked at the vectorizer change here, but this code is not needed. Before computing costs, we collect the uniform values in collectLoopUniforms(). ICmp instructions of the kind here are marked uniform. Then in getInstructionCost(), we check if an instruction is uniform (isUniformAfterVectorization()), and if so, always return "1" for the cost, regardless of VF.
> > 
> > Also, floating-point induction variables aren't allowed to be "primary" induction variables. So it shouldn't be the case that you would have a FCmp feeding the back edge branch.
> Correction: we always return the cost of the *scalar* compare if it is uniform, regardless of VF.
Thanks for explaining - I removed this from patch.


https://reviews.llvm.org/D29631