Use the pure attribute for the vload, vload_half and vloada_half builtins.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
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.
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.
Apologies for the late reply... I'd prefer to get the langref updated first, for the sake of consistency and to ensure other stakeholders agree with the interpretation. You can still go ahead with the __attribute__((pure)) changes of course.
Thanks! I have updated the review to use __attribute__((pure)) only (i.e. it no longer uses __attribute__((const)).
looks like this broke tests: http://45.33.8.238/linux/63308/step_7.txt
Please take a look, and revert for now if it takes a while to fix.
I've updated the review to include test changes that are required for check-clang-semaopencl to pass.