[PATCH] D35156: [ARM] Make ARMExpandPseudo add implicit uses for predicated instructions

[Eli Friedman via Phabricator via llvm-commits]

efriedma added a comment.

OK, I'll add a testcase.



================
Comment at: lib/Target/ARM/ARMExpandPseudoInsts.cpp:1081
+          .add(MI.getOperand(4))
+          .add(makeImplicit(MI.getOperand(1)));
 
----------------
rovka wrote:
> How exactly does this work? You're already passing the register from MI.getOperand(1) to BuildMI, which IIUC will mark it as a define. Why do you also need to make it implicit?
We also need to mark it as a use (the instruction is predicated, so if the predicate is false, the value isn't modified).

It's an implicit use because it isn't encoded anywhere in the instruction.


================
Comment at: lib/Target/ARM/ARMExpandPseudoInsts.cpp:1135
+          .add(MI.getOperand(4))
+          .add(makeImplicit(MI.getOperand(1)));
       MI.eraseFromParent();
----------------
rovka wrote:
> IIUC, MOVi16 is AI1, not AsI1. Doesn't that mean it shouldn't set any CC, so you don't need an implicit operand for it at all?
It doesn't modify CPSR.  It uses CPSR ("MI.getOperand(4)").  I'm not sure how that's relevant to this patch, though.


Repository:
  rL LLVM

https://reviews.llvm.org/D35156