[PATCH] D76550: [Attributor] Improve the alignment of the loads

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added a reviewer: courbet.
lebedev.ri added a subscriber: courbet.
lebedev.ri added a comment.

@courbet To be honest

  explicit MaybeAlign(uint64_t Value) {
    assert((Value == 0 || llvm::isPowerOf2_64(Value)) &&
           "Alignment is neither 0 nor a power of 2");
    if (Value)
      emplace(Value);
  }

that if-check looks like a major rake.



================
Comment at: llvm/lib/Transforms/IPO/Attributor.cpp:5638-5642
+          // Whenever no alignment available for the load instruction we invoke
+          // the natural alignment.
+          createReplacementValues(MaybeAlign(AlignAA.getAssumedAlign() == 0
+                                                 ? 1
+                                                 : AlignAA.getAssumedAlign()),
----------------
I'd suggest rephrasing this to explain what is going on, something like
```
// Alignment of 0 is treated by MaybeAlign as no alignment,
// but when no alignment is specified for the load instruction,
// natural alignment is assumed. So we have to manually bump it to 1.
createReplacementValues(MaybeAlign(std::max(1, AlignAA.getAssumedAlign()),
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76550