On Monday 14 September 2009 12:22, Dan Gohman wrote:> Hi Dave, > > On Sep 11, 2009, at 3:31 PM, David Greene wrote: > > Attached is a patch to print asm comments for spill information. > > We've discussed the mechanisms before but I wanted to run the > > patch by everyone before I start to commit pieces. > > The Offset->FrameIndex mapping seems rather heavy-weight, as > any expense is incurred even when AsmVerbose is off. Would it > be possible to use MachineMemOperands instead? In theory, > they should already be preserving the needed information. If > they're not sufficient, could they be improved?Yeah, I'm not totally happy with that mapping either. With MachineMemOperands, would that be the getOffset() method? That's only for FPRel data, though. What if frame pointer elimination has been run? I would love to get out from under the map if possible.> There is work going on to improve the register allocator's ability > to use rematerlialization. In a world where the register allocator > can aggressively remat misc. loads, will it still be interesting to > identify spill reloads?Yes. I have a later patch that marks remats as well. The idea is to get a sense of how well regalloc is doing. And you can't remat everything.> You mentioned at one point that you have some scripts which > know how to read some of these comments. Would it be > possible for you to include these scripts with the patch so that > others can consider your patch in context?I'm not sure. The script counts a whole bunch of things. All it does is look for these commentsw and tally up statistics, what's in loops, etc. It's pretty straightforward. -Dave
On Monday 14 September 2009 12:32, David Greene wrote:> > The Offset->FrameIndex mapping seems rather heavy-weight, as > > any expense is incurred even when AsmVerbose is off. Would it > > be possible to use MachineMemOperands instead? In theory, > > they should already be preserving the needed information. If > > they're not sufficient, could they be improved? > > Yeah, I'm not totally happy with that mapping either. With > MachineMemOperands, would that be the getOffset() method? That's > only for FPRel data, though. What if frame pointer elimination has > been run? > > I would love to get out from under the map if possible.Are MachineMemOperands available at AsmPrinter time? Where are they stashed? -Dave
On Sep 14, 2009, at 10:34 AM, David Greene wrote:> On Monday 14 September 2009 12:32, David Greene wrote: > > >>> The Offset->FrameIndex mapping seems rather heavy-weight, as >>> >>> any expense is incurred even when AsmVerbose is off. Would it >>> >>> be possible to use MachineMemOperands instead? In theory, >>> >>> they should already be preserving the needed information. If >>> >>> they're not sufficient, could they be improved? >>> >> >> >> Yeah, I'm not totally happy with that mapping either. With >> >> MachineMemOperands, would that be the getOffset() method? That's >> >> only for FPRel data, though. What if frame pointer elimination has >> >> been run? >> >> >> >> I would love to get out from under the map if possible. >> > > Are MachineMemOperands available at AsmPrinter time? Where are they > stashed?Yes. There are some places where they aren't preserved, but otherwise they stick around for the entire CodeGen process. The places that don't preserve them should ideally be fixed eventually. They're available via the memoperands_begin()/memoperands_end() MachineInstr member functions. Dan
On Sep 14, 2009, at 10:32 AM, David Greene wrote:> On Monday 14 September 2009 12:22, Dan Gohman wrote: > >> Hi Dave, >> >> >> >> On Sep 11, 2009, at 3:31 PM, David Greene wrote: >> >>> Attached is a patch to print asm comments for spill information. >>> >>> We've discussed the mechanisms before but I wanted to run the >>> >>> patch by everyone before I start to commit pieces. >>> >> >> >> The Offset->FrameIndex mapping seems rather heavy-weight, as >> >> any expense is incurred even when AsmVerbose is off. Would it >> >> be possible to use MachineMemOperands instead? In theory, >> >> they should already be preserving the needed information. If >> >> they're not sufficient, could they be improved? >> > > Yeah, I'm not totally happy with that mapping either. With > MachineMemOperands, would that be the getOffset() method? That's > only for FPRel data, though. What if frame pointer elimination has > been run? > > I would love to get out from under the map if possible.MachineMemOperands for spill slots use FixedStack PseudoSourceValues as their base. There's a unique FixedStack PseudoSourceValue for each fixed frame object, so it's independent of whether frame pointer elimination has been done, and it's independent of the actual frame offsets. Dan
On Monday 14 September 2009 13:07, Dan Gohman wrote:> MachineMemOperands for spill slots use FixedStack PseudoSourceValues > as their base. There's a unique FixedStack PseudoSourceValue for each > fixed frame object, so it's independent of whether frame pointer > elimination has been done, and it's independent of the actual frame > offsets.>From MachineMemOperand.h:/// getValue - Return the base address of the memory access. /// Special values are PseudoSourceValue::FPRel, PseudoSourceValue::SPRel, /// and the other PseudoSourceValue members which indicate references to /// frame/stack pointer relative references and other special references. const Value *getValue() const { return V; } I don't see PseudoSourceValue::FPRel, etc. defined anywhere. How do I know if a PseudoSourceValue is from the stack? -Dave