[PATCH] D63938: [ARM] Stop using scalar FP instructions in integer-only MVE mode.

Tue Jul 2 03:52:34 PDT 2019

dmgreen added inline comments.

================
Comment at: llvm/test/CodeGen/Thumb2/float-ops.ll:268
+; ONLYREGS: lsls    r2, r2, #31
+; ONLYREGS: vmovne.f32      s2, s0
 ; HARD: lsls    r0, r0, #31
----------------
dmgreen wrote:
> simon_tatham wrote:
> > dmgreen wrote:
> > > This is worse than before by the looks of it? We move things into fp registers just to move them out again.
> > Perhaps that's true, but I'd rather fix the correctness first and get a set of regression tests passing, and //then// we can worry about recovering the performance with those tests in place to prevent breaking correctness again.
> > 
> > Especially since in the general case it's not really clear how you //should// choose which kind of register to use for a value. Keeping it in an FP register is obviously wasteful in this case, but in another case where register pressure is high, might it save a spill or two? I think getting the right answers in cases larger than this simple one might not be trivial.
> My worry is that this will mean every floating point load becomes a vldr, which just ends up being moved into a gpr. This probably isn't a huge deal for performance, as you will likely always be calling a __aeabi_fadd type function, but the codesize would increase quite a bit. I couldn't think of a reason when you _would_ want to load using a vldr (at least it would be fairly uncommon). I imagine that almost every operation would actually be done on a gpr for floats.
Forgot to say. I agree that working is better than not working. Something we may have to adjust in the future though.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63938