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

[Diana Picus via Phabricator via llvm-commits]

rovka added a comment.

Hi Eli,

I couldn't find an awful lot of tests for ARM pseudo expansion in particular, so I think it would be nice to add one. A MIR test checking each of the transformations that you touched in this patch would work fine.

I've been looking into this area myself recently and found it all a bit confusing. I have a few questions inline, sorry if they're a bit basic.

Thanks.



================
Comment at: lib/Target/ARM/ARMExpandPseudoInsts.cpp:1081
+          .add(MI.getOperand(4))
+          .add(makeImplicit(MI.getOperand(1)));
 
----------------
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?


================
Comment at: lib/Target/ARM/ARMExpandPseudoInsts.cpp:1135
+          .add(MI.getOperand(4))
+          .add(makeImplicit(MI.getOperand(1)));
       MI.eraseFromParent();
----------------
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?


Repository:
  rL LLVM

https://reviews.llvm.org/D35156