On Wed, Sep 19, 2012 at 09:04:30PM +0200, Dag-Erling Sm??rgrav wrote: > Konstantin Belousov <kostik...@gmail.com> writes: > > "Dag-Erling Sm??rgrav" <d...@des.no> writes: > > > I assume you mean assignments, not calculations. I trust the compiler > > > to move them to where they are needed - a trivial optimization with SSA. > > It is a dereference before the assignment, so I perceive it as the > > calculation. No, I do not think that compiler can move the dereference, > > because there are many sequential points between the calculation and > > use of the result. > > Sequence points are a language feature and are only meaningful in the > translation phase. Once the code is in SSA form or some other > equivalent intermediate representation, the compiler can see that the > variables are only used in one specific case and move the assignments > inside that block. In fact, it will probably optimize them away, > because they are completely unnecessary - I added them solely for > readability after Niclas called my attention to the fact that it is > almost impossible to understand fdgrowtable() at a first reading. Compiler cannot change the semantic of the program regardless of the phase of compilation the change happens at.
Compiler that would reorder reads from the global memory across sequential points seems to be not disallowed by c99, but it certainly contradicts to our semantic of the lock releases, which the compiler cannot infer from the non-inlined function calls. Malloc call is the sequential point and does have the release semantic FWIW. > > > > Correct, thanks for pointing it out. The easiest solution is to place > > > the struct freetable between the file array and the flag array. > > As I know, for all ABIs FreeBSD run on, the structure alignment is the > > alignment of the most requiring member. You would introduce very tacit > > dependency between struct file and struct freetable. > > The existing code *already* places the struct freetable immediately > after the struct file array. What I'm proposing now is to leave the > struct freetable where it was but move the fileflags array so they don't > overlap. The fileflags array is actually a char[] and has no alignment > requirement. Ok. > > Memory usage will not increase, because the code already allocates > additional space for the struct freetable to make sure it will fit even > if onfiles < sizeof(struct freetable). > > BTW, I just noticed that there is some dead code in fdgrowtable(): > > nnfiles = NDSLOTS(nfd) * NDENTRIES; /* round up */ > if (nnfiles <= onfiles) > /* the table is already large enough */ > return; > > /* ... */ > > /* allocate new bitmaps if necessary */ > if (NDSLOTS(nnfiles) > NDSLOTS(onfiles)) > nmap = malloc(NDSLOTS(nnfiles) * NDSLOTSIZE, > M_FILEDESC, M_ZERO | M_WAITOK); > else > nmap = NULL; > > Since neither nnflags nor onflags are modified between these two chunks > of code, the condition in the second if will always be true. You mean that new bitmap shall be always allocated, making the nmap = NULL assignment not needed ? I agree. It would also make the code in the last if () executed unconditionally.
pgpxF1S7g8LNi.pgp
Description: PGP signature
