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

[Dave Green via Phabricator via llvm-commits]

dmgreen added a comment.

In D76847#1945904 <https://reviews.llvm.org/D76847#1945904>, @samparker wrote:

> I'm surprised not to see more test changes, does this really have no effect on any existing tests?


This kind of code can come up from intrinsics a lot, but not necessarily from the relatively simple codegen tests we have. Someone can write intrinsics knowing that VPNOT's should be folded into a single VPT block, only for llvm to come along and nicely optimize them all away, producing IR that we end up with worse codegen for. This should help us get back to better assembly.

More testing does sound good.



================
Comment at: llvm/lib/Target/ARM/MVEVPTOptimisationsPass.cpp:167-168
+  MIBuilder.add(MachineOperand::CreateReg(Target, /*isDef*/ false));
+  MIBuilder.addImm(0);
+  MIBuilder.addReg({});
+  InstrOperand.setReg(NewResult);
----------------
addUnpredicatedMveVpredNOp


================
Comment at: llvm/lib/Target/ARM/MVEVPTOptimisationsPass.cpp:233
+    // on "VPNOTOperand".
+    for (MachineBasicBlock::instr_iterator Iter = ++Instr.getIterator();
+         Iter != MBB.end(); ++Iter) {
----------------
Should this not be replacing _all_ uses of the old condition with the value from the VPNOT?

As in, if we see input code like:
a = VCMP ..
use(a)
b = VPNOT a
..
use(b)
..
use(a)

Could it not be generally beneficial to insert a VPNOT between use of b and the second use of a? We could also probably have any amount of code between the predicated uses and it might still be beneficial, considering the costs of spills/reloads vs a single VPNOT.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76847