[PATCH] D86074: [ARM][MVE] Tail-predication: check get.active.lane.mask's TC value

[Sam Parker via Phabricator via llvm-commits]

samparker added inline comments.


================
Comment at: llvm/lib/Target/ARM/MVETailPredication.cpp:390
+
+  if ((ConstElemCount = dyn_cast<ConstantInt>(ElemCount))) {
+    ConstantInt *TC = dyn_cast<ConstantInt>(TripCount);
----------------
nit: unnecessary parenthesis nesting.


================
Comment at: llvm/lib/Target/ARM/MVETailPredication.cpp:422
+    // most cases though.
+    const SCEVAddRecExpr *AddRecExpr;
+
----------------
It looks like this can be sunk into the if-statement that defines it.


================
Comment at: llvm/lib/Target/ARM/MVETailPredication.cpp:424
+
+    if (dyn_cast<SCEVAddExpr>(EC) || dyn_cast<SCEVUnknown>(EC)) {
+      // If the element count is a simple AddExpr or SCEVUnknown, which is e.g.
----------------
isa<>


================
Comment at: llvm/lib/Target/ARM/MVETailPredication.cpp:428
+      // if it is an operand in the tripcount scev expression.
+      if (dyn_cast<SCEVAddExpr>(TC) && !SE->hasOperand(TC, EC)) {
+        LLVM_DEBUG(dbgs() << "ARM TP: Can't verify the element counter\n");
----------------
isa<>


================
Comment at: llvm/lib/Target/ARM/MVETailPredication.cpp:432
+      }
+    } else if ((AddRecExpr = dyn_cast<SCEVAddRecExpr>(EC))) {
+      // For more complicated AddRecExpr, check that the corresponding loop and
----------------
nit: parenthesis


================
Comment at: llvm/lib/Target/ARM/MVETailPredication.cpp:439
+      }
+    }
+  }
----------------
I guess we should return false for any other SCEVExpr type?


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

https://reviews.llvm.org/D86074