[LLVMdev] Mixing noalias and regular arguments

[Nick Lewycky]

Kuperstein, Michael M wrote:
> Ping?

Pong! Sorry for the slow review, I had this patch starred but hadn't got 
around to it. Yes, the rationale and implementation are correct.

> (Is there a code owner for AA, btw?)

(It falls back on the more general code owner who is Chris Lattner in 
this case, "Everything not covered by someone else".)

+/// isNoAliasArgument - Return true if this is an argument with the noalias
+/// attribute.
+bool isNoAliasArgument(const Value* V);

"const Value* V" should be "const Value *V".

+    // Arguments can't alias with noalias arguments
+    if ((isa<Argument>(O1) && isNoAliasArgument(O2)) ||
+        (isa<Argument>(O2) && isNoAliasArgument(O1)))
+      return NoAlias;

Fold this into the logic right above it:

     // Arguments can't alias with local allocations or noalias calls
     // in the same function.
     if (((isa<Argument>(O1) && (isa<AllocaInst>(O2) || 
isNoAliasCall(O2))) ||
          (isa<Argument>(O2) && (isa<AllocaInst>(O1) || 
isNoAliasCall(O1)))))
       return NoAlias;

by factoring out the combined tests "isa<AllocaInst>(V) || 
isNoAliasCall(V) || isNoAliasArgument(V)" into a function.

+/// isNoAliasArgument - Return true if this is an argument with the noalias
+/// attribute.
+bool llvm::isNoAliasArgument(const Value* V)
+{
+  if (const Argument *A = dyn_cast<Argument>(V))

Please be consistent, "const Value* V" vs. "const Argument *A". This 
file puts the star on the right of the space.

Thanks for fixing this. Please commit once you've addressed the above!

Nick

> *From:*llvmdev-bounces at cs.uiuc.edu [mailto:llvmdev-bounces at cs.uiuc.edu]
> *On Behalf Of *Kuperstein, Michael M
> *Sent:* Tuesday, May 21, 2013 18:23
> *To:* LLVMdev at cs.uiuc.edu
> *Cc:* Raoux, Thomas F
> *Subject:* [LLVMdev] Mixing noalias and regular arguments
>
> Hi all,
>
> I’m trying to understand the semantics of noalias arguments, and I’m not
> entirely sure I got it correctly.
>
> To the best of my understanding, if an argument is declared noalias,
> “This indicates that pointer values based on the argument do not alias
> pointer values which are not based on it” implies, among other things,
> that it cannot alias any other argument, even if that argument is NOT
> declared noalias.
>
> However, currently, BasicAliasAnalysis doesn’t recognize this case
> explicitly. Sometimes it will work for other reasons (e.g. if it knows
> the other argument does not get captured), but it’s relatively easy to
> get circumstances where the result is MayAlias.
>
> I’m attaching a patch that addresses this.
>
> Can anyone offer an opinion on the basic issue and, assuming this is the
> desired behavior, on the patch?
>
> Thanks,
>
> Michael