[PATCH] D95288: [ValueTracking] Don't assume readonly function will return

[Florian Hahn via Phabricator via llvm-commits]

fhahn accepted this revision.
fhahn added a comment.
This revision is now accepted and ready to land.

LGTM, thanks



================
Comment at: lib/Analysis/ValueTracking.cpp:5059
-    // is guaranteed to return.
-    return CB->onlyReadsMemory() || CB->onlyAccessesArgMemory();
   }
----------------
I guess we could incorporate the `argmemonly` reasoning in 'FunctionAttrs`, using something like `argmemonly + nosync => willreturn`, assuming that IO can only happen through inaccessible memory? Just `argmemonly` would not be enough I think, because we still need to rule out volatile accesses/synchronization via. the arguments. 


================
Comment at: unittests/Analysis/ValueTrackingTest.cpp:654
       false, // call void @unknown(i32* %p) argmemonly
       true,  // call void @nounwind_willreturn(i32* %p)
       false, // ret void
----------------
also add `// call void @unknown(i32* %p) willreturn`?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D95288/new/

https://reviews.llvm.org/D95288