[PATCH] D12838: [GlobalsAA] Teach GlobalsAA about memory intrinsics

[Hal Finkel via llvm-commits]

----- Original Message -----
> From: "James Molloy" <james at jamesmolloy.co.uk>
> To: reviews+D12838+public+3fb78cda248f2b96 at reviews.llvm.org, "Sanjoy Das" <sanjoy at playingwithpointers.com>, "james
> molloy" <james.molloy at arm.com>, chandlerc at gmail.com, "david majnemer" <david.majnemer at gmail.com>, hfinkel at anl.gov
> Cc: llvm-commits at lists.llvm.org
> Sent: Monday, September 14, 2015 11:29:42 AM
> Subject: Re: [PATCH] D12838: [GlobalsAA] Teach GlobalsAA about memory intrinsics
> 
> Hi Sanjoy,
> 
> The lazy lambda idea is a good one, I'll implement that, thanks.
> 
> readonly/readnone: Yes, I think so. That'd be a simple addition.
> 
> Hal - I'm not certain I understand why having nocapture better
> supported is a more general issue than this and not an orthogonal
> one. Nocapture can tell us about the escapability of a pointer,
> sure, and what you suggest seems a sensible addition to GMR (allows
> us to catch more cases of non-escaping private globals). But this
> patch is really looking at mod/ref behavior, and nocapture can't
> help us there.
> 
> Were you suggesting it as a general improvement or something you'd
> specifically like to see in this patch?

For this patch. This is relevant because the memcpy/memmove/memset intrinsics's arguments are tagged as 'nocapture', and arguably, that should be enough. Let's make it enough.

 -Hal

> 
> Cheers,
> 
> 
> James
> 
> 
> On Mon, 14 Sep 2015 at 17:19 Sanjoy Das via llvm-commits <
> llvm-commits at lists.llvm.org > wrote:
> 
> 
> sanjoy added a subscriber: sanjoy.
> 
> ================
> Comment at: lib/Analysis/GlobalsModRef.cpp:771
> @@ +770,3 @@
> + SmallVector<bool,4> Operands;
> + for (auto &U : CS.args())
> + Operands.push_back(GetUnderlyingObject(&*U, DL) == GV);
> ----------------
> I'd rather have this be a lambda that takes an argument index instead
> of an eagerly populated vector, given that for most intrinsics this
> isn't used.
> 
> ================
> Comment at: lib/Analysis/GlobalsModRef.cpp:776
> @@ +775,3 @@
> + default:
> + return MRI_ModRef;
> + case Intrinsic::memset:
> ----------------
> aadg wrote:
> > Shouldn't we also handle the isVolatile argument ? As anything
> > could happen when this is a volatile operation, I think we should
> > always return MRI_ModRef when it is set.
> Can we do something more precise here if the intrinsic is readonly or
> readnone?
> 
> 
> Repository:
> rL LLVM
> 
> http://reviews.llvm.org/D12838
> 
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
> 

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