On Fri, Jul 31, 2020 at 11:50 AM Andres Freund <and...@anarazel.de> wrote: > I hope the above makes this make sene now? It's about subsequent uses of > the StringInfo, rather than the body of resetStringInfo itself.
That does make sense, except that https://en.cppreference.com/w/c/language/restrict says "During each execution of a block in which a restricted pointer P is declared (typically each execution of a function body in which P is a function parameter), if some object that is accessible through P (directly or indirectly) is modified, by any means, then all accesses to that object (both reads and writes) in that block must occur through P (directly or indirectly), otherwise the behavior is undefined." So my interpretation of this was that it couldn't really affect what happened outside of the function itself, even if the compiler chose to perform inlining. But I think see what you're saying: the *inference* is only valid with respect to restrict pointers in a particular function, but what can be optimized as a result of that inference may be something further afield, if inlining is performed. Perhaps we could add a comment about this, e.g. Marking these pointers with pg_restrict tells the compiler that str and str->data can't overlap, which may allow the compiler to optimize better when this code is inlined. For example, it may be possible to keep str->data in a register across consecutive appendStringInfoString operations. Since pg_restrict is not widely used, I think it's worth adding this kind of annotation, lest other hackers get confused. I'm probably not the only one who isn't on top of this. > > In appendStringInfoChar, why do we need to cast to restrict twice? Can > > we not just do something like this: > > > > char *pg_restrict ep = str->data+str->len; > > ep[0] = ch; > > ep[1] = '\0'; > > I don't think that'd tell the compiler that this couldn't overlap with > str itself? A single 'restrict' can never (?) help, you need *two* > things that are marked as not overlapping in any way. But what's the difference between: + *(char *pg_restrict) (str->data + str->len) = ch; + str->len++; + *(char *pg_restrict) (str->data + str->len) = '\0'; And: char *pg_restrict ep = str->data+str->len; ep[0] = ch; ep[1] = '\0'; ++str->len; Whether or not str itself is marked restricted is another question; what I'm talking about is why we need to repeat (char *pg_restrict) (str->data + str->len). I don't have any further comment on the remainder of your reply. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company