[PATCH] D151870: [ARM] Increase cost of unaligned double and float loads

[Sjoerd Meijer via Phabricator via llvm-commits]

SjoerdMeijer added a comment.

Just nits, lgtm too.

One question about my favourite fp16 data type while we are at it. Does or can that benefit from some handling too here?

And one last question about this:

> // Unaligned float loads introduce an extra store and a register move
> // The codegen for these unaligned loads could be improved,

Was just curious if this is visible in an existing test and if there's a FIXME/TODO as a comment.



================
Comment at: llvm/lib/Target/ARM/ARMTargetTransformInfo.cpp:1498
+  // Unaligned double loads introduce 3 extra stores (1 to the stack), 2 loads
+  // and a register move
+  // Unaligned float loads introduce an extra store and a register move
----------------
Nit: punctation.


================
Comment at: llvm/lib/Target/ARM/ARMTargetTransformInfo.cpp:1501
+  // The codegen for these unaligned loads could be improved, but these costs
+  // represent what is currently produced
+  if (ST->hasFPRegs64() && Src->isDoubleTy() && Alignment && Alignment.valueOrOne() < 8)
----------------
Same


================
Comment at: llvm/lib/Target/ARM/ARMTargetTransformInfo.cpp:1502
+  // represent what is currently produced
+  if (ST->hasFPRegs64() && Src->isDoubleTy() && Alignment && Alignment.valueOrOne() < 8)
+    return 6;
----------------
Do you need to check Alignment (` .. && Alignment && ..`) when you use `valueOrOne()`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151870