[PATCH] D76847: [Target][ARM] Replace re-uses of old VPR values with VPNOTs

[Dave Green via Phabricator via llvm-commits]

dmgreen added inline comments.


================
Comment at: llvm/lib/Target/ARM/MVEVPTOptimisationsPass.cpp:250
+      if (!IsWritingToVCCR(*Iter) ||
+          getVPTInstrPredicate(*Iter) != ARMVCC::None)
+        continue;
----------------
We not do this for predicated VCMP's?


================
Comment at: llvm/lib/Target/ARM/MVEVPTOptimisationsPass.cpp:257
+      if (VCCRValue && Iter->getOpcode() == ARM::MVE_VPNOT &&
+          Iter->findRegisterUseOperand(VCCRValue)) {
+        // Move the VPNOT closer to its first user if needed, and ignore if it
----------------
findRegisterUseOperandIdx != -1


================
Comment at: llvm/lib/Target/ARM/MVEVPTOptimisationsPass.cpp:269
+      // Else, just set VCCRValue if it's a VCMP.
+      if (IsVCMP(Iter->getOpcode()))
+        VCCRValue = Dst;
----------------
What makes us not do this for other opcodes? (I guess vcmp is the vast majority of cases?)


================
Comment at: llvm/lib/Target/ARM/MVEVPTOptimisationsPass.cpp:286
+
+    // The second loop tries to optimize the remaining instructions. It'll stop
+    // early if it finds an unpredicated VCMP that it doesn't know about.
----------------
The second loop -> This second loop, just to be a little clearer.


================
Comment at: llvm/lib/Target/ARM/MVEVPTOptimisationsPass.cpp:302
+          MachineInstrBuilder MIB =
+              BuildMI(MBB, &*Iter, Iter->getDebugLoc(), TII->get(ARM::COPY))
+                  .addDef(CopyResult)
----------------
Do we need to add the copy? We could just use LastVPNOTResult directly? Or would that be more complex?


================
Comment at: llvm/lib/Target/ARM/MVEVPTOptimisationsPass.cpp:305
+                  .addReg(LastVPNOTResult);
+          DeadInstructions.push_back(&*Iter);
+
----------------
Set Modified = true here too I think.


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

https://reviews.llvm.org/D76847