[PATCH] D66483: [ARM] Generate 8.1-m CSINC, CSNEG and CSINV instructions.

Sat Aug 24 03:31:49 PDT 2019

dmgreen marked an inline comment as done.
dmgreen added a comment.

Sorry for the delay. I found a problem which was proving difficult to reduce, but I'm now pretty sure that is just exposing some problem in loloops.

> Could you give a brief description of why this is better than just using an "it" block?

The tests shows this comes up more than I had expected. In general, we are replacing a IT; MVN/ADD/RSB with a single CSINV/CSINC/CSNEG, which can also use zr and includes a "free move". For codesize the IT pair will be either 4 bytes or 6, depending on the instruction needed and what registers are used. The CS* will always be 4, and includes that free move.  For cycle times, we are talking about cortex-m here so I would expect the IT may or may not be free depending on if the surrounding instructions can be dual issued. The CS* should always take a single cycle I would presume. So generally smaller or the same, which is what my other testing also shows.

================
Comment at: llvm/test/CodeGen/Thumb2/mve-vcmpfr.ll:769
+; CHECK-MVE-NEXT:    vcmpe.f32 s1, s4
+; CHECK-MVE-NEXT:    csinc r1, zr, zr, eq
 ; CHECK-MVE-NEXT:    vmrs APSR_nzcv, fpscr
----------------
efriedma wrote:
> This is cset, right?  Why aren't we printing the alias?
It looks like this is because the instruction has an implicit CPSR use, which the Alias isn't accounting for. It's only checking for 4 operands, but during codegen the MI instruction will have 5.

I'll split this into a separate patch. It's not a big change but it's probably better split out from here.

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

https://reviews.llvm.org/D66483