[PATCH] D34188: Don't check side effects for functions outside of SCoP

[Sameer AbuAsal via Phabricator via llvm-commits]

sabuasal marked an inline comment as done.
sabuasal added inline comments.


================
Comment at: lib/Support/SCEVValidator.cpp:121
 bool polly::isConstCall(llvm::CallInst *Call) {
   if (Call->mayReadOrWriteMemory())
     return false;
----------------
sabuasal wrote:
> singam-sanjay wrote:
> > grosser wrote:
> > > singam-sanjay wrote:
> > > > For polyhedral analysis, do we also need the function to evaluate to the same value always (not just side effect free) inside the Scop ? I got this notion from the name of the function.
> > > > 
> > > > If that's the case, is it possible to check for that ? ( I have a feeling that it's not feasible )
> > > Yes, that's what we need. I believe that we can assume that if a function does not read memory it will always return the same value.
> > > 
> > > However, there is another bug that slipped in. I assumed mayReadOrWriteMemory corresponds to is-always-executed. This is not the case, so we should likely limit this to functions that are called in the entry block (or at least post-dominate the entry block).
> > > Yes, that's what we need. I believe that we can assume that if a function does not read memory it will always return the same value.
> > 
> > What about random number generators ?
> > 
> > > However, there is another bug that slipped in. I assumed mayReadOrWriteMemory corresponds to is-always-executed. This is not the case, so we should likely limit this to functions that are called in the entry block (or at least post-dominate the entry block).
> > 
> > How does mayReadOrWrite become mustReadOrWrite for those functions ?
> > 
> > 
> > 
> Well, random number generators actually modify some object internally to keep trak of the series of random numbers generated so by definition they are not const functions.
Oh for the second part, in this test case I am we only have declarations of the function g() not the definition so we can't really tell. 


Repository:
  rL LLVM

https://reviews.llvm.org/D34188