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

[Florian Hahn via Phabricator via llvm-commits]

fhahn added inline comments.


================
Comment at: llvm/lib/Analysis/ValueTracking.cpp:5046
+    // annotated.
+    return isa<IntrinsicInst>(CB) && CB->onlyReadsMemory();
   }
----------------
SjoerdMeijer wrote:
> Drive by comment: if this reads volatile memory, it is not side-effect free? If this is a check for "side-effect free intrinsics", does this check cover that?
Unfortunately this is not completely clear from the `readonly` definition, but I don't think to would currently be safe to treat functions with volatile or atomic loads as `readonly`. They are currently treated as may-write-to-memory, see https://llvm.org/docs/LangRef.html#volatile-memory-accesses

We have `nosync` to model some of those aspects separately, but it is not really used at the moment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95288