[PATCH] D110742: [OpenCL] Add pure and const attributes to vload builtins

Stuart Brady via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Oct 8 03:46:30 PDT 2021


stuart added a comment.

In D110742#3048301 <https://reviews.llvm.org/D110742#3048301>, @svenvh wrote:

>> For the constant address space, the const attribute (or readnone) can be used. As memory in the constant address space is immutable, the statement in the langref that: "if a readnone function reads or writes memory visible to the program, or has other side-effects, the behavior is undefined" does not apply. The reading of immutable memory does not have side-effects, nor can it be affected by side-effects.
>
> I think `readnone` might be too strong, because the pointer argument will still be dereferenced (while `readnone` implies that "the function computes its result [...] based strictly on its arguments, without dereferencing any pointer arguments").

That may be so, but the function does not have its own side-effects, nor can it depend upon any side-effects, as the memory in question is truly immutable (i.e. it cannot even be changed by another process, thread, or shared memory interface in such a way that would be observable).

All functions, with or without `__attribute__((const))` can be said to require memory reads in the form of the instruction fetches needed to execute the instructions that comprise the compiled function. It is also quite normal for constant pools to be generated for a function, from which memory reads will be performed despite no pointer being dereferenced in the original C source.

Both `__attribute__((const))` and `readnone` are intended to permit arbitrary code motion, and so in my opinion the langref misstates the true nature of the `readnone` attribute. (The Clang manual does not even document `__attribute__((const))`.) From a cursory examination of the LLVM source, I did not find any use of `readnone` that conflicts with this interpretation of the `readnone` attribute.

Does the langref need to be amended, first, or is it okay to interpret the `readnone` attribute as it was clearly intended, without going through the process of updating the langref first?

I can update this review to use `__attribute__((pure))` for all address spaces, for the time being, but it seems a shame that the poor wording in the langref might (necessarily) prevent us from making the optimal change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110742



More information about the cfe-commits mailing list