On Sat, 24 Sep 2011, David Miller wrote: > From: Hans-Peter Nilsson <h...@bitrange.com> > Date: Sat, 24 Sep 2011 17:15:06 -0400 (EDT) > > I'd prefer it as a parameter to the builtins (expanding to two > > insns, letting gcc get rid of the redundant ones; let the > > initialization value be 0). I understand you're trying to keep > > some kind of compatibility there, but additional builtins would > > do the trick and fit nicely: the new builtins expanding to a set > > of GSR (GSR field) followed by the "old" insn but fixed as in > > this patch. Besides, the functions that use GSR still can't be > > const in this patch. I guess they never can, when you think of > > it, setting and/or using a register that can affect/be affected > > something elsewhere, when that something is known to gcc. Oh well. > > I read this idea in your PR before I did this work and I disagree that > this is a better approach, because then I have to assume that you care > about all the other bits in the %gsr register.
I don't understand what you mean here. Maybe it doesn't matter... My suggestions come from observing what gcc did to the "faked gsr modelling" I had to use with the current releases (what moving and eliminating redundant variable settings used in asms that it did; turned out acceptable FWIW, no redundant reads), which would map directly to my suggestion. But I guess you have a point in that your setting-gsr-then-using-builtins maps better to the machine insns. BTW, don't forget to clobber GSR at call insns! > So on the first set I'd have to read it, mask it out, then set the > scale bits. A needless waste of 20 to 30 cycles on UltraSPARC-III. No, it doesn't have to be read. If the fields have (useful) implicit initial values (like scale=7 and align=4) at the beginning of any function, you wouldn't have to read and mask, just set. (Caveat: the port has to have a way to emit a gsr-setting even if the supposed-initial-values are specified - like another register or variable, or the initial-value machinery as I suggested.) > If you just call "__vis_write_gsr()" at the beginning of your kernel, > you can tell the compiler that you just want to set the scaling bits > and you don't care about the others at all. Don't care how? They're certainly set by both __vis_write_gsr() and alignaddr and used by faligndata. I guess my confusion is that I don't see what aspect is "don't care" here that'd be "care" with my suggestion. > > Another aspect would be to model the different GSR fields as > > different registers; they're used completely differently and > > just happen to be set with the same insn. That might help gcc > > getting rid of redundant settings. > > Again, this doesn't allow the user to say "don't care" about the other > fields like a plain "__vis_write_gsr(2<<3)" call does. But that'd set GSR.alignaddr_offset to 0 rather than "don't care". > You know what fields actually matter for your code. A good reason to model them as different registers! Still, this is a good start and much more workable (and schedulable) than what's already there, thank you for that. It doesn't add hurdles for a revisit, if the mechanism is found unusable or the generated code pessimal! brgds, H-P
