[llvm-commits] [PATCH] Address Space Alias Analysis

[Tobias Grosser]

On 10/11/2011 04:08 PM, Justin Holewinski wrote:
> The attached patch provides a new alias analysis: Address Space Alias
> Analysis.  This pass is off-by-default and has no impact on generated
> code unless -asaa is provided to opt.
>
> This analysis signals NoAlias for pointers in different address spaces.
>   The primary target for this pass is GPU back-ends which use LLVM
> address spaces as a means for differentiating memory types (on-chip
> shared, off-chip global, etc.) and hence pointers in different address
> spaces cannot alias.  For OpenCL code compiled with the PTX back-end,
> this improves performance by over 2x in some of my test kernels due to
> LLVM using this alias information to remove redundant loads.
>
> Is this okay to commit?

Hey Justin,

thanks for the patch. I have one remark on the code itself:

> +    virtual AliasResult alias(const Location&LocA, const Location&LocB) {
> +      const Value *V1 = LocA.Ptr;
> +      const Value *V2 = LocB.Ptr;
> +
> +
> +      // The logic here is very simple: pointers to two different address spaces
> +      // cannot alias.
> +      if (V1->getType()->isPointerTy()&&  V2->getType()->isPointerTy()) {
> +        const PointerType *PT1 = dyn_cast<const PointerType>(V1->getType());
> +        const PointerType *PT2 = dyn_cast<const PointerType>(V2->getType());
> +
> +        if (PT1->getAddressSpace() != PT2->getAddressSpace()) {
> +          DEBUG(dbgs()<<  "ASAA: NoAlias -<"<<  *V1<<  ">,<"<<  *V2<<  ">\n");
> +          return NoAlias;
> +        }
> +      }

Can you simplify this to the following code?

virtual AliasResult alias(const Location&LocA, const Location&LocB) {
   const PointerType *PT1 = dyn_cast<const 
PointerType>(LocA.Ptr->getType());
   const PointerType *PT2 = dyn_cast<const 
PointerType>(LocA.Ptr->getType());

   // The logic here is very simple: pointers to two different address
   // spaces cannot alias.
   if (PT1 && PT2)
     if (PT1->getAddressSpace() != PT2->getAddressSpace()) {
       DEBUG(dbgs()<<  "ASAA: NoAlias -<"<<  *V1<<  ">,<"<<  *V2<<  ">\n");
       return NoAlias;
     }
   }
   [..]

Otherwise, the code is pretty straightforward and should be OK to
commit. As I have limited experience with the alias analysis framework, give
other people two days to veto (or to 'okay' it entirely). If no problems
are raised, please commit.

Cheers
Tobi