[PATCH] D74935: [LangRef][AliasAnalysis] Clarify `noalias` affects only modified objects

This revision was automatically updated to reflect the committed changes. Closed by commit rG6cd673345cfa: [LangRef][AliasAnalysis] Clarify `noalias` affects only modified objects (authored by jdoerfert). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D74935: [LangRef][AliasAnalysis] Clarify `noalias` affects only modified objects

jdoerfert added a comment. In D74935#1915311 , @jeroen.dobbelaere wrote: > In D74935#1909909 , @jdoerfert wrote: > > > I would say that once we get modeling for indirect restrict we can adapt > > the lang ref

[PATCH] D74935: [LangRef][AliasAnalysis] Clarify `noalias` affects only modified objects

jeroen.dobbelaere added a comment. In D74935#1909909 , @jdoerfert wrote: > I would say that once we get modeling for indirect restrict we can adapt the > lang ref accordingly. For now there is only have outer level restrict/noalias. Why not try to get

[PATCH] D74935: [LangRef][AliasAnalysis] Clarify `noalias` affects only modified objects

jdoerfert added a comment. In D74935#1909054 , @jeroen.dobbelaere wrote: > In D74935#1908665 , @jdoerfert wrote: > > > I think we conflate two things here: > > > > 1. The modifications to the LangRef which should

[PATCH] D74935: [LangRef][AliasAnalysis] Clarify `noalias` affects only modified objects

jeroen.dobbelaere added a comment. In D74935#1908665 , @jdoerfert wrote: > I think we conflate two things here: > > 1. The modifications to the LangRef which should be in accordance with the C > standard (at least I haven't seen you contradict the new

[PATCH] D74935: [LangRef][AliasAnalysis] Clarify `noalias` affects only modified objects

jdoerfert added a comment. In D74935#1908607 , @jeroen.dobbelaere wrote: > [...] > So, adding the 'noalias' attribute to %pA and %pB will change the behavior > of foo(). > > hmm... this explanation is maybe getting too long ? > The short conclusion

[PATCH] D74935: [LangRef][AliasAnalysis] Clarify `noalias` affects only modified objects

jeroen.dobbelaere added a comment. In D74935#1907939 , @jdoerfert wrote: > In D74935#1907100 , > @jeroen.dobbelaere wrote: > > > Just to give an example: > > > > int foo(int* restrict *pA, int* restrict *pB) { >

[PATCH] D74935: [LangRef][AliasAnalysis] Clarify `noalias` affects only modified objects

jdoerfert added a comment. In D74935#1907100 , @jeroen.dobbelaere wrote: > Just to give an example: > > int foo(int* restrict *pA, int* restrict *pB) { > int tmp=**pB; > **pA=42; > return tmp - **pB; // **pA and **pB can refer to the same

[PATCH] D74935: [LangRef][AliasAnalysis] Clarify `noalias` affects only modified objects

jeroen.dobbelaere added a comment. Just to give an example: int foo(int* restrict *pA, int* restrict *pB) { int tmp=**pB; **pA=42; return tmp - **pB; // **pA and **pB can refer to the same objects } This gives following llvm-ir code with the 'full noalias' clang version (after

[PATCH] D74935: [LangRef][AliasAnalysis] Clarify `noalias` affects only modified objects

jdoerfert added a comment. In D74935#1905026 , @jeroen.dobbelaere wrote: > The reason I bring this up, is that in the full restrict implementation > (D68484 ), the alias analysis will do a > recursive analysis, proving

[PATCH] D74935: [LangRef][AliasAnalysis] Clarify `noalias` affects only modified objects

jeroen.dobbelaere added a comment. The reason I bring this up, is that in the full restrict implementation (D68484 ), the alias analysis will do a recursive analysis, proving that *pA and *pB will never alias, because pA and pB will never alias. But this

[PATCH] D74935: [LangRef][AliasAnalysis] Clarify `noalias` affects only modified objects

jeroen.dobbelaere added a comment. Be aware that c99 restrict has a special case for nested restrict that makes the rules stronger: void f(int* restrict *restrict pA, int* restrict * restrict pB) { **pA=**pB; } although pA and pB are only read, (c99 6.7.3.1 #4) specifies that, writing

[PATCH] D74935: [LangRef][AliasAnalysis] Clarify `noalias` affects only modified objects

efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. Current language seems fine. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74935/new/ https://reviews.llvm.org/D74935

[PATCH] D74935: [LangRef][AliasAnalysis] Clarify `noalias` affects only modified objects

aqjune added a comment. I agree readnone functions can read constant memory; at least LangRef doesn't restrict readnone functions from accessing memory (unless it changes a state visible to caller). I see that readnone is also attached to a function that loads from/stores to alloca:

[PATCH] D74935: [LangRef][AliasAnalysis] Clarify `noalias` affects only modified objects

jdoerfert added a comment. In D74935#1888506 , @arsenm wrote: > Another related point I’ve never been clear on is if a readnone function is > allowed to read constant memory Right, ... I basically did assume something for the Attributor readnone

[PATCH] D74935: [LangRef][AliasAnalysis] Clarify `noalias` affects only modified objects

arsenm added a comment. Another related point I’ve never been clear on is if a readnone function is allowed to read constant memory Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74935/new/ https://reviews.llvm.org/D74935

[PATCH] D74935: [LangRef][AliasAnalysis] Clarify `noalias` affects only modified objects

jdoerfert added a comment. In D74935#1887960 , @nhaehnle wrote: > In D74935#1887245 , @efriedma wrote: > > > In D74935#1886020 , @nhaehnle > > wrote: > > > > > I find this

[PATCH] D74935: [LangRef][AliasAnalysis] Clarify `noalias` affects only modified objects

jdoerfert updated this revision to Diff 246088. jdoerfert added a comment. Update language to memory locations instead of objects Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74935/new/ https://reviews.llvm.org/D74935 Files:

[PATCH] D74935: [LangRef][AliasAnalysis] Clarify `noalias` affects only modified objects

nhaehnle added a comment. In D74935#1887245 , @efriedma wrote: > In D74935#1886020 , @nhaehnle wrote: > > > I find this phrasing pretty confusing. How about the following: > > > > > This indicates that objects

[PATCH] D74935: [LangRef][AliasAnalysis] Clarify `noalias` affects only modified objects

efriedma added a comment. In D74935#1886020 , @nhaehnle wrote: > I find this phrasing pretty confusing. How about the following: > > > This indicates that objects accessed via pointer values based on the > > argument or return value are not **modified**,

[PATCH] D74935: [LangRef][AliasAnalysis] Clarify `noalias` affects only modified objects

nlopes added a comment. I agree with @aqjune that stating clearly the definition of object in this context. See this example in the C spec: For example, the second call of f in g has undefined behavior because each of d[1] through d[49] is accessed through both p and q. void g(void) {

[PATCH] D74935: [LangRef][AliasAnalysis] Clarify `noalias` affects only modified objects

nhaehnle added a comment. I find this phrasing pretty confusing. How about the following: > This indicates that objects accessed via pointer values based on the argument > or return value are not **modified**, during the execution of the function, > via pointer values not based on the argument

[PATCH] D74935: [LangRef][AliasAnalysis] Clarify `noalias` affects only modified objects

jdoerfert updated this revision to Diff 245795. jdoerfert added a comment. Remove leftover word Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74935/new/ https://reviews.llvm.org/D74935 Files: llvm/docs/LangRef.rst Index: llvm/docs/LangRef.rst

[PATCH] D74935: [LangRef][AliasAnalysis] Clarify `noalias` affects only modified objects

aqjune added a comment. Never mind, I see that the keyword object comes from the alias analysis document. The alias analysis document is using the term object consistently. I was confused because the term is used at llvm.objectsize intrinsic's LangRef documentation, which seems to refer an

[PATCH] D74935: [LangRef][AliasAnalysis] Clarify `noalias` affects only modified objects

jdoerfert updated this revision to Diff 245774. jdoerfert added a comment. Tried to simplify the wording by making it two sentences. @efriedma's wording is another option. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74935/new/

[PATCH] D74935: [LangRef][AliasAnalysis] Clarify `noalias` affects only modified objects

aqjune added a comment. Hi, The term 'object' seems ambiguous to me. For example, void f(i32* noalias x, i32* noalias y) { *x = 1; *y = 2; } int x[2]; f([0], [1]); If object means an allocation, this should be UB, because x and y are pointing to the same object. Memory

[PATCH] D74935: [LangRef][AliasAnalysis] Clarify `noalias` affects only modified objects

efriedma added a comment. I'm not sure I like the wording; it's a complicated sentence to understand. Can we make it sort of list-like? "for each memory location accessed through a noalias pointer, either: 1. the location is not modified during the execution of the function. 2. all accesses

[PATCH] D74935: [LangRef][AliasAnalysis] Clarify `noalias` affects only modified objects

jdoerfert created this revision. jdoerfert added reviewers: hfinkel, nlopes, regehr, efriedma. Herald added a subscriber: bollu. Herald added a reviewer: sstefan1. Herald added a reviewer: uenoku. Herald added a project: LLVM. We already mention that `noalias` is modeled after the C99 `restrict`