[PATCH] D76925: [Alignment][NFC] MachineMemOperand::getAlign/getBaseAlign

[Guillaume Chatelet via Phabricator via llvm-commits]

gchatelet marked 4 inline comments as done.
gchatelet added inline comments.


================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:13653
   // aligned and the type is a vector with elements up to 4 bytes
-  if (Subtarget.needsSwapsForVSXMemOps() && !(MMO->getAlignment()%16)
-      && VecTy.getScalarSizeInBits() <= 32 ) {
+  if (Subtarget.needsSwapsForVSXMemOps() && MMO->getAlign() >= Align(16) &&
+      VecTy.getScalarSizeInBits() <= 32) {
----------------
courbet wrote:
> This is not an NFC. Should this be `isAligned()` ?
`isAligned` currently works the other way around, here is the definition

```
inline bool isAligned(Align Lhs, uint64_t SizeInBytes) {
  return SizeInBytes % Lhs.value() == 0;
}
```

Rewritten with `isAligned`, `!(MMO->getAlignment()%16` gives `isAligned(Align(16), MMO->getAlign().value())`, this is not very readable.

Now `!(A % 16)` means that `A` should be a multiple of 16.
Since `A` is a power of two by definition it is a multiple of 16 iff it is `>=16`.

So although it is not a straightforward it is NFC.



================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:13724
   // aligned and the type is a vector with elements up to 4 bytes
-  if (Subtarget.needsSwapsForVSXMemOps() && !(MMO->getAlignment()%16)
-      && VecTy.getScalarSizeInBits() <= 32 ) {
+  if (Subtarget.needsSwapsForVSXMemOps() && MMO->getAlign() >= Align(16) &&
+      VecTy.getScalarSizeInBits() <= 32) {
----------------
courbet wrote:
> ditto
same here


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76925