[PATCH] D71521: Support for library function aligned_alloc

[Uday Bondhugula via Phabricator via llvm-commits]

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


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp:4300
+                      Attribute::getWithDereferenceableOrNullBytes(
+                          Call.getContext(), Op1C->getZExtValue()));
   } else if (isReallocLikeFn(&Call, TLI) && Op1C) {
----------------
jdoerfert wrote:
> bondhugula wrote:
> > jdoerfert wrote:
> > > If the alignment is constant we should add a align attribute as well.
> > Actually, it need not be! :-) The alignment with the aligned_alloc call can be dynamic - it's just that we convert it to a stack allocation only when it's constant.
> > Actually, it need not be! :-) The alignment with the aligned_alloc call can be dynamic - it's just that we convert it to a stack allocation only when it's constant.
> 
> I am aware.
> 
> Please read my comment as:
>   For the cases where the alignment is a constant, which we can determine here since we are looking at the actual call, we should ...
> 
> I was not expecting this to be ambiguous given that the code you added will also only trigger if the size is a constant. Same thing, but with alignment is missing. 
But should we be adding an attribute for something that by itself is already a constant operand? A pass can't possibly rely on the attribute and would anyway need to look at the operand. More importantly, if the alignment operand is updated (although there isn't such a case), the attribute would have to be kept in sync. So, do we need such redundancy? (dereferenceable_or_null attr is different in all these regards.)


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

https://reviews.llvm.org/D71521