Re: CVS commit: src/sys/arch/sparc64/include
On Sat, Apr 06, 2019 at 09:40:15PM +, Takeshi Nakayama wrote: > Module Name: src > Committed By: nakayama > Date: Sat Apr 6 21:40:15 UTC 2019 > > Modified Files: > src/sys/arch/sparc64/include: psl.h > > Log Message: > The real cause for removing asm inline code on clang is that the > "r" constraint cannot handle 64-bit and is treated as 32-bit. > > So code that refers to the upper 32-bit (manuf or impl) of the %ver > register is removed by optimization. > > Use 32-bit kernel code as a workaround when referring to the %ver > register. Thanks! Could you please add a note to doc/TODO.clang about the hackaround? Joerg
Re: CVS commit: src/sys/arch/sparc64/include
On Sat, Apr 06, 2019 at 07:05:22AM +0900, Takeshi Nakayama wrote: > >>> Joerg Sonnenberger wrote > > > On Fri, Apr 05, 2019 at 12:15:41PM +, Takeshi Nakayama wrote: > > > Module Name: src > > > Committed By: nakayama > > > Date: Fri Apr 5 12:15:41 UTC 2019 > > > > > > Modified Files: > > > src/sys/arch/sparc64/include: psl.h > > > > > > Log Message: > > > Put "memory" to asm inline reading privilege registers for clang to > > > prevent it from being removed by excessive optimization. > > > > Why excessive optimization? With the the clobber or volatile, the asm > > statement is declared to be removable. > > With? Without? > > Excessive may be overstated, but in the case of gcc it is not > removed, so I thought so. Sorry, I meant "without". Assembler constraints are nasty to debug, since they often work for years... Joerg
Re: CVS commit: src/sys/arch/sparc64/include
>>> Joerg Sonnenberger wrote > On Fri, Apr 05, 2019 at 12:16:13PM +, Takeshi Nakayama wrote: > > Module Name:src > > Committed By: nakayama > > Date: Fri Apr 5 12:16:13 UTC 2019 > > > > Modified Files: > > src/sys/arch/sparc64/include: ctlreg.h > > > > Log Message: > > Add dummy constraints to avoid excessive optimization in clang. > > GENERIC kernel compiled with clang now boot at least on my Fire V100. > > The constraint still seems to be wrong to me. The store variant should > list the content as output constraint, not as input? Thanks for looking > at this. When I put it in the output constraint, the code size increased in the case of gcc, so I put it in the input constraint for now. However, I think so too that it should be put in the output constraint, so I will change it. -- Takeshi Nakayama
Re: CVS commit: src/sys/arch/sparc64/include
>>> Joerg Sonnenberger wrote > On Fri, Apr 05, 2019 at 12:15:41PM +, Takeshi Nakayama wrote: > > Module Name:src > > Committed By: nakayama > > Date: Fri Apr 5 12:15:41 UTC 2019 > > > > Modified Files: > > src/sys/arch/sparc64/include: psl.h > > > > Log Message: > > Put "memory" to asm inline reading privilege registers for clang to > > prevent it from being removed by excessive optimization. > > Why excessive optimization? With the the clobber or volatile, the asm > statement is declared to be removable. With? Without? Excessive may be overstated, but in the case of gcc it is not removed, so I thought so. -- Takeshi Nakayama
Re: CVS commit: src/sys/arch/sparc64/include
On Fri, Apr 05, 2019 at 12:15:41PM +, Takeshi Nakayama wrote: > Module Name: src > Committed By: nakayama > Date: Fri Apr 5 12:15:41 UTC 2019 > > Modified Files: > src/sys/arch/sparc64/include: psl.h > > Log Message: > Put "memory" to asm inline reading privilege registers for clang to > prevent it from being removed by excessive optimization. Why excessive optimization? With the the clobber or volatile, the asm statement is declared to be removable. Joerg
Re: CVS commit: src/sys/arch/sparc64/include
On Fri, Apr 05, 2019 at 12:16:13PM +, Takeshi Nakayama wrote: > Module Name: src > Committed By: nakayama > Date: Fri Apr 5 12:16:13 UTC 2019 > > Modified Files: > src/sys/arch/sparc64/include: ctlreg.h > > Log Message: > Add dummy constraints to avoid excessive optimization in clang. > GENERIC kernel compiled with clang now boot at least on my Fire V100. The constraint still seems to be wrong to me. The store variant should list the content as output constraint, not as input? Thanks for looking at this. Joerg
Re: CVS commit: src/sys/arch/sparc64/include
On Tue, Dec 30, 2014 at 04:34:42PM -0600, Dennis Ferguson wrote: On 30 Dec, 2014, at 12:52 , David Laight da...@l8s.co.uk wrote: Is that the correct fix? Unless the rdpr actually accesses memory (don't think it does) then then problem is probably a missing 'volatile' instead. Certainly the way those asm functions are defined looks to be rather more obfuscated than necessary. Or maybe just get rid of the __constfunc? I think that, plus the (void) argument list to the inline, is explicitly telling the compiler it should feel free to move the call absolutely anywhere it feels like placing it. The relevant optimisations happen after the inline, so the function definition shouldn't affect things. I'm guessing that __constfunc just means that the compiler can merge multiple calls (and cache the result), not at all sure that is valid with a memory clobber. I'm a bit surprised the memory clobber by itself changed anything at all since I thought a memory clobber without a 'volatile' is supposed to refer only to memory-based input and output arguments to the asm(), of which there are none. The compiler doesn't know which args to the asm reference memory unless you tell it with an appropriate 'clobber' - either the global one, or a ranged one based on one if the arguments. In this case I suspect removing __constfunc and making the asm volatile will force correct sequencing. David -- David Laight: da...@l8s.co.uk
Re: CVS commit: src/sys/arch/sparc64/include
On Wed, Dec 31, 2014 at 10:05:22AM +, David Laight wrote: In this case I suspect removing __constfunc and making the asm volatile will force correct sequencing. Check the commit history. Can we make only the hypverisor call __constfunc? Martin
Re: CVS commit: src/sys/arch/sparc64/include
On Wed, Dec 31, 2014 at 11:15:24AM +0100, Martin Husemann wrote: On Wed, Dec 31, 2014 at 10:05:22AM +, David Laight wrote: In this case I suspect removing __constfunc and making the asm volatile will force correct sequencing. Check the commit history. Can we make only the hypverisor call __constfunc? Looks like the two changes are fighting in different directions. One is trying to get the compiler to cache the result of the function by calling it once at the top, the other to ensure it only called when some other condition is true. You can't have it both ways. Maybe that one function shoukd be caching the result of the CPU_IS_* macro. Or maybe save the relevant register value in memory during initialisation. David -- David Laight: da...@l8s.co.uk
Re: CVS commit: src/sys/arch/sparc64/include
Michael Lorenz macal...@netbsd.org wrote Module Name: src Committed By: macallan Date: Thu Jan 9 12:51:27 UTC 2014 Modified Files: src/sys/arch/sparc64/include: cpu.h Log Message: allow non-SUN4V kernels to build I think that fixing sparc64/genassym.cf is a porper fix, but this change is also necessary as mrg pointed out. -- Takeshi Nakayama
