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

[Diana Picus via Phabricator via llvm-commits]

rovka added a comment.

Thanks for the test-case and the explanations.
LGTM but you should probably wait for someone more versed in these things to give the final approval.



================
Comment at: lib/Target/ARM/ARMExpandPseudoInsts.cpp:1135
+          .add(MI.getOperand(4))
+          .add(makeImplicit(MI.getOperand(1)));
       MI.eraseFromParent();
----------------
efriedma wrote:
> 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.
Sorry, I don't know where I was going with that, the code looks perfectly fine. 


================
Comment at: test/CodeGen/ARM/expand-pseudos.mir:69
+
+# CHECK-LABEL: name: test1
+# CHECK: %r1 = MOVi16 500, 0, killed %cpsr, implicit killed %r1
----------------
If you want to move the checks closer to the code, you can use ; CHECK (instead of # CHECK).


Repository:
  rL LLVM

https://reviews.llvm.org/D35156