[PATCH] D80072: Make Value::getPointerAlignment() return an Align, not a MaybeAlign.

[Eli Friedman via Phabricator via llvm-commits]

efriedma marked an inline comment as done.
efriedma added inline comments.


================
Comment at: llvm/lib/Analysis/Loads.cpp:40
+      BA = DL.getABITypeAlign(Ty);
   }
+  const APInt APAlign(Offset.getBitWidth(), Alignment.value());
----------------
jdoerfert wrote:
> As you noted, this seems wrong. I think this allows wrong results for `llvm::isDereferenceableAndAlignedPointer` queries. What happens if you remove this boosting? 
> 
> --- 
> 
> From D9791: 
> > I assume that pointers without explicit alignment have ABI alignment.
> 
> I think this is OK for pointers accessed by loads without specified alignment but not for "pointers" in the general sense. If you look at the AttributorAttributes.cpp change, the function there does something similar to the `getBaseAlign` on the left but it only does so for pointers accessed without specified alignment. (= I think I came to this conclusion for the second time.)
The simplest way to show what happens is to just post the patch, so I did that.  The issue primarily involves pointers marked with the dereferenceable metadata/attribute; we used to assume they were naturally aligned, but now we don't.

Frontends adding dereferenceable attributes/metadata should also add the corresponding alignment attributes/metadata, but I'm not sure that's actually happening consistently in practice.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80072