[PATCH] D69945: [ARM][MVE] Tail predication conversion

[Sam Parker via Phabricator via llvm-commits]

samparker marked 3 inline comments as done.
samparker added inline comments.


================
Comment at: llvm/lib/Target/ARM/ARMLowOverheadLoops.cpp:48
+    bool Revert = false;
+    bool TailPredicate = false;
+    bool CannotTailPredicate = false;
----------------
SjoerdMeijer wrote:
> Looks like this `TailPredicate` is a boolean for "FoundVPT", so can we just use `VCTP`? Or perphaps at least change the name because it might be easy to confuse it with `CannotTailPredicate`?
We need to track whether we've found only one vctp instruction, so I need some flag. But yes, a rename sounds good.


================
Comment at: llvm/lib/Target/ARM/ARMLowOverheadLoops.cpp:426
+          LoLoop.ScanForVPR(&MI);
+        LoLoop.CheckTPValidity(&MI);
       }
----------------
SjoerdMeijer wrote:
> If we cannot tail-predicate, does that mean we keep checking TPValidity here? I think this is place where the usage of `CannotTailPredicate` and `TailPredicate` can be a bit confusing.
I'll rename it to FoundOneVCTP. We'll look for vpr users only once we've found the vctp, but we need to check all the instructions in the loop so that we catch any of our unsupported vector instructions.


================
Comment at: llvm/test/CodeGen/Thumb2/LowOverheadLoops/vector-arith-codegen.ll:392
+; Function Attrs: nofree norecurse nounwind
+define dso_local arm_aapcs_vfpcc void @vector_mul_vector_u8(i8* noalias nocapture %a, i8* noalias nocapture readonly %b, i8* noalias nocapture readonly %c, i32 %N) local_unnamed_addr #0 {
+; CHECK-LABEL: vector_mul_vector_u8:
----------------
SjoerdMeijer wrote:
> If I'm not mistaken, function `@vector_mul_vector_u8` and `@vector_mul_vector_s8` above are exactly the same?
Ah yes, because there's no extending going on. I'll remove the duplicates.


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

https://reviews.llvm.org/D69945