[PATCH] Use align parameter attribute for all pointer arguments

[Philip Reames]

Same.  LGTM.

I take it you're going to canonicalize assumptions about alignment as 
align attributes where possible?

Philip

On 07/22/2014 09:31 AM, Chandler Carruth wrote:
>
> LGTM, seems like obvious goodness to me.
>
> On Jul 22, 2014 9:04 AM, "hfinkel at anl.gov <mailto:hfinkel at anl.gov>" 
> <hfinkel at anl.gov <mailto:hfinkel at anl.gov>> wrote:
>
>     Hi chandlerc, reames, aschwaighofer, nadav,
>
>     This is almost more of a documentation change than anything else...
>
>     We currently support the align attribute on all (pointer)
>     parameters, but we only use it for byval parameters. However, it
>     is completely consistent at the IR level to treat 'align n' on all
>     pointer parameters as an alignment assumption on the pointer. This
>     patch does this:
>
>      1. Causes computeKnownBits to use the align attribute on all
>     pointer parameters, not just byval parameters.
>      2. Updated the LangRef to document the align parameter attribute
>     (as it turns out, it was not documented at all previously,
>     although the byval documentation mentioned that it could be used).
>
>     There are two benefits to doing this:
>      1. It allows enhancing alignment based on the pointer alignment
>     after inlining
>      2. It allows simplification of pointer arithmetic
>
>     My primary use case for these things involves pointers that are
>     over-aligned to allow for efficient vectorized code generation.
>
>     Thanks again!
>
>     http://reviews.llvm.org/D4620
>
>     Files:
>       docs/LangRef.rst
>       lib/Analysis/ValueTracking.cpp
>       test/Bitcode/attributes.ll
>       test/Transforms/InstCombine/align-attr.ll
>
>     Index: docs/LangRef.rst
>     ===================================================================
>     --- docs/LangRef.rst
>     +++ docs/LangRef.rst
>     @@ -921,6 +921,13 @@
>          the first parameter. This is not a valid attribute for return
>          values.
>
>     +``align <n>``
>     +    This indicates that the pointer value may be assumed by the
>     optimizer to
>     +    have the specified alignment.
>     +
>     +    Note that this attribute has additional semantics when
>     combined with the
>     +    ``byval`` attribute.
>     +
>      .. _noalias:
>
>      ``noalias``
>     Index: lib/Analysis/ValueTracking.cpp
>     ===================================================================
>     --- lib/Analysis/ValueTracking.cpp
>     +++ lib/Analysis/ValueTracking.cpp
>     @@ -308,13 +308,9 @@
>        }
>
>        if (Argument *A = dyn_cast<Argument>(V)) {
>     -    unsigned Align = 0;
>     +    unsigned Align = A->getType()->isPointerTy() ?
>     A->getParamAlignment() : 0;
>
>     -    if (A->hasByValOrInAllocaAttr()) {
>     -      // Get alignment information off byval/inalloca arguments
>     if specified in
>     -      // the IR.
>     -      Align = A->getParamAlignment();
>     -    } else if (TD && A->hasStructRetAttr()) {
>     +    if (!Align && TD && A->hasStructRetAttr()) {
>            // An sret parameter has at least the ABI alignment of the
>     return type.
>            Type *EltTy =
>     cast<PointerType>(A->getType())->getElementType();
>            if (EltTy->isSized())
>     Index: test/Bitcode/attributes.ll
>     ===================================================================
>     --- test/Bitcode/attributes.ll
>     +++ test/Bitcode/attributes.ll
>     @@ -239,6 +239,11 @@
>              ret i8* %a
>      }
>
>     +define void @f41(i8* align 32, double* align 64) {
>     +; CHECK: define void @f41(i8* align 32, double* align 64) {
>     +        ret void
>     +}
>     +
>      ; CHECK: attributes #0 = { noreturn }
>      ; CHECK: attributes #1 = { nounwind }
>      ; CHECK: attributes #2 = { readnone }
>     Index: test/Transforms/InstCombine/align-attr.ll
>     ===================================================================
>     --- /dev/null
>     +++ test/Transforms/InstCombine/align-attr.ll
>     @@ -0,0 +1,15 @@
>     +; RUN: opt < %s -instcombine -S | FileCheck %s
>     +target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
>     +target triple = "x86_64-unknown-linux-gnu"
>     +
>     +; Function Attrs: nounwind uwtable
>     +define i32 @foo1(i32* align 32 %a) #0 {
>     +entry:
>     +  %0 = load i32* %a, align 4
>     +  ret i32 %0
>     +
>     +; CHECK-LABEL: @foo1
>     +; CHECK-DAG: load i32* %a, align 32
>     +; CHECK: ret i32
>     +}
>     +
>
>     _______________________________________________
>     llvm-commits mailing list
>     llvm-commits at cs.uiuc.edu <mailto:llvm-commits at cs.uiuc.edu>
>     http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140722/4e5e325c/attachment.html>