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

[Nikita Popov via Phabricator via llvm-commits]

nikic added a comment.

In D102348#2755385 <https://reviews.llvm.org/D102348#2755385>, @etiotto wrote:

> 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.

No, it does not allow reading/writing arguments. That would be inaccessibleorargmemonly.

> 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 ?

That depends on the precise semantics `@llvm.annotation` is supposed to have, with which I'm not familiar, and which LangRef does not specify particularly clearly. Is it okay to drop an `@llvm.annotation()` whose return value is not used? If "no", then marking it readonly is not possible, it will be dropped as dead code. Is it okay to hoist `@llvm.annotation()` inside an unconditional loop header out of the loop? If "no", then dropping inaccessiblememonly is not possible either (being non-speculatable would prevent hoisting out of a conditional loop header, but not an unconditional one).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102348