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

[Mark Schimmel via Phabricator via cfe-commits]

marksl accepted this revision.
marksl added a comment.
This revision is now accepted and ready to land.

Looks good


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107611

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Mark Schimmel via Phabricator via cfe-commits]

marksl added inline comments.



Comment at: llvm/lib/Target/ARC/ARCExpandPseudos.cpp:86
+  //   %R2 = RSUB_cc_rru6 %R2, 31, pred:2, %STATUS
+  MachineInstr  = *SII;
+  const MachineOperand  = 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

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits