[PATCH] D107611: [ARC] Add codegen for llvm.ctlz intrinsic for the ARC backend

[Mark Schimmel via Phabricator via llvm-commits]

marksl added inline comments.


================
Comment at: llvm/lib/Target/ARC/ARCExpandPseudos.cpp:86
+  //	%R2<def,tied1> = RSUB_cc_rru6 %R2<tied0>, 31, pred:2, %STATUS<imp-use>
+  MachineInstr &SI = *SII;
+  const MachineOperand &Dest = SI.getOperand(0);
----------------
I know you're following ExpandStore above in using "SI" here, but let's switch to "MI" as that is the most common practice. I think he used "SI" because it was a store instruction. All your readers will recognize MI as a MachineInstr, but they will have no such association to SI.


================
Comment at: llvm/lib/Target/ARC/ARCExpandPseudos.cpp:93
+
+  BuildMI(*SI.getParent(), SI, SI.getDebugLoc(), TII->get(ARC::FLS_f_rr),
+          DestReg)
----------------
This is not right. The final instruction of the sequence must write to DestReg.


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

https://reviews.llvm.org/D107611