[PATCH] D102348: [Instructions]: Calls marked with inaccessiblememonly attribute should be considered to not read/write memory

[Ettore Tiotto via Phabricator via llvm-commits]

etiotto added a comment.

In D102348#2755223 <https://reviews.llvm.org/D102348#2755223>, @nikic wrote:

> This looks wrong to me. To give one example: Accessing inaccessible memory is the typical way to model side-effects. mayHaveSideEffects() is defined in terms of mayWriteToMemory(). This change makes inaccessiblememonly side-effect free. Oops.
>
> I do think that "only writes accessible memory" is a useful predicate, and many -- but not all -- current callers of mayWriteToMemory() can be switched to it. I would recommend to introduce mayWriteAccessibleMemory() / mayReadAccessibleMemory() and then migrate call-sites after review of the guarantees they actually need.

OK. So the semantics the `inaccessiblememonly` attribute allows the function to read or write memory pointed to by an argument.  This PR was motivated by wanting to make `llvm.annotation` be consider side-effects free. That intrinsic is defined as:

  def int_annotation : DefaultAttrsIntrinsic<
      [llvm_anyint_ty],
      [LLVMMatchType<0>, llvm_ptr_ty, llvm_ptr_ty, llvm_i32_ty],
      [IntrInaccessibleMemOnly], "llvm.annotation">;

and documented as:

  The first argument is an integer value (result of some expression), the second is a pointer to a global string, the third is a pointer to a global string which is the source file name, and the last argument is the line number. It returns the value of the first argument.

So the intrinsic can read the arguments but it should not be able to modify their values. Nor does it need to modify global memory.  So would marking that intrinsic with the `readonly` attribute be acceptable ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102348