[PATCH] D76971: Deduce attributes for aligned_alloc in InstCombine
Uday Bondhugula via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Mar 30 09:11:05 PDT 2020
bondhugula added inline comments.
================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp:4445
+ Attribute::getWithDereferenceableOrNullBytes(
+ Call.getContext(), Op1C->getZExtValue()));
} else if (isReallocLikeFn(&Call, TLI) && Op1C) {
----------------
jdoerfert wrote:
> bondhugula wrote:
> > jdoerfert wrote:
> > > bondhugula wrote:
> > > > jdoerfert wrote:
> > > > > When Op0C is set (regardless of Op1C), we can set the alignment attribute on the return value, can't we?
> > > > Would it be correct to set the alignment attribute for 0 sized allocations?
> > > > Would it be correct to set the alignment attribute for 0 sized allocations?
> > >
> > > Not in general, but: `The function aligned_alloc() is the same as memalign(), except for the added restriction that size should be a multiple of alignment.` So for a non-zero alignment size need to be non-zero. Please correct me if I'm wrong though.
> > The size could still be zero. (zero as size would be a multiple of any alignment. :-))
> > The size could still be zero. (zero as size would be a multiple of any alignment. :-))
>
> True, it was late.
>
> Then remove the "(regardless of Op1C),") from my original comment. We still can and should add alignment information.
Sure, we should. I was just making sure under what circumstances we could. Will add this, thanks.
================
Comment at: llvm/test/Transforms/InstCombine/deref-alloc-fns.ll:39
+ ret i8* %call
+}
+
----------------
jdoerfert wrote:
> I mean, we can add `align 32` here can't we not?
Absolutely!
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D76971/new/
https://reviews.llvm.org/D76971
More information about the llvm-commits
mailing list