[PATCH] Use align parameter attribute for all pointer arguments

[Hal Finkel]

----- Original Message -----
> From: "Philip Reames" <listmail at philipreames.com>
> To: "Chandler Carruth" <chandlerc at gmail.com>, reviews+D4620+public+5c0fe6b45bf029a8 at reviews.llvm.org
> Cc: "Nadav Rotem" <nrotem at apple.com>, hfinkel at anl.gov, llvm-commits at cs.uiuc.edu, aschwaighofer at apple.com
> Sent: Tuesday, July 22, 2014 11:55:35 AM
> Subject: Re: [PATCH] Use align parameter attribute for all pointer arguments
> 
> 
> Same. LGTM.
> 
> I take it you're going to canonicalize assumptions about alignment as
> align attributes where possible?

Yes, when we get that far :-)

 -Hal

> 
> 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 " < 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
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> 
> 
> 

-- 
Hal Finkel
Assistant Computational Scientist
Leadership Computing Facility
Argonne National Laboratory