[PATCH] D90935: [ARM][LowOverheadLoops] Merge VCMP and VPST across VPT blocks

[Dave Green via Phabricator via llvm-commits]

dmgreen added inline comments.


================
Comment at: llvm/lib/Target/ARM/ARMLowOverheadLoops.cpp:1568
+      MachineInstr *VPST = Insts.front();
+      auto Next = ++MachineBasicBlock::iterator(VPST);
+      assert(getVPTInstrPredicate(*Next) != ARMVCC::None &&
----------------
If Next is only use in an assert, be careful that it does not cause a warning in release mode.


================
Comment at: llvm/lib/Target/ARM/ARMLowOverheadLoops.cpp:1587
+          // This assertion protects against changed to that behaviour
+          assert(!IntermediateInstrsUseVPR &&
+                 "Instructions between the VCMP and VPST are not expected to "
----------------
Same if this is only used in an assert.

I'm still not 100% sure that this will always hold true with VPNOT or VPSEL (or anything else in the future as things change. They are not supported in TP loops yet but theoretically could be and things like this could easily with how subtle they are).

It would probably be better as an if, to be honest. It's only a minor loss of perf if it does happen, and that way it's safer.


================
Comment at: llvm/lib/Target/ARM/ARMLowOverheadLoops.cpp:1621
+          assert(!IntermediateInstrsUseVPR && "Instructions between the VCMP and VPST are not expected to be predicated");
+          ReplaceVCMPWithVPT(VCMP, VPST);
+          LLVM_DEBUG(dbgs() << "ARM Loops: Removing VPST: " << *VPST);
----------------
samtebbs wrote:
> dmgreen wrote:
> > Do we need to do the same thing to remove kill flags as D90964?
> Hmm I'm not sure. From what I've seen of the pass' output. the kill flags are still valid when the instruction is removed.
It would be something like:
  VPST 4
  VLDR $vpr
  VCMP q0, q1, $vpr
  VORR q2, killed q1, $noreg
  VPST 8
  VORR ... $vpr
The unpredicated VORR kills the register that the VCMP will still use.


================
Comment at: llvm/test/CodeGen/Thumb2/LowOverheadLoops/vcmp-vpst-combination-across-blocks.mir:197
+    renamable $q2 = MVE_VORR renamable $q1, renamable $q1, 1, renamable $vpr, killed renamable $q2
+    MVE_VPST 4, implicit $vpr
+    renamable $q1 = MVE_VORR killed renamable $q1, renamable $q0, 1, renamable $vpr, renamable $q1
----------------
This can be `MVE_VPST 8` for a single instruction block.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90935