On Monday 13 July 2009 11:40, Chris Lattner wrote:> > I was attempting to reduce the number of files affected, but if you > > want this change I'll go ahead and do it. > > Makes sense, thanks. Please do it as a separate patch from the other > changes though since it will be large and mechanical.Ok, no problem.> > - Tag instructons with source line information (customers really > > want this). > > Right, that would be nice. This should be synthesizable from the > DebugLoc on the instruction in the asm printer, no need to redundantly > encode it into the comment field.Except the DebugLoc stuff isn't all there yet AFAIK. We've been using the comment mechanism for over a year. I agree we should move to synthesizing it from DebugLoc when it's ready. We're not going to submit our line number stuff anyway (it's too much of a hack) but we would like the comment infrastructure to be there.> > - Tag instructions as register spills or reloads. > > I'm not sure what this means exactly, but would it be sufficient for > the asmprinter to use isLoadFromStackSlot and print this if the FI is > a spill slot?Maybe. I'm not sure what information is available here. The other thing this code does is tag it as a vector or scalar spill/reload. Synthesizing that might be trickier as you have to take the opcode into account. It's a lot of work, at the very least.> > - Tag instructions with an ID of the tblgen pattern that generated > > them. This > > is super useful for debugging. > > this would also be really nice :). This can be generated by the asm > printer as well.How so? Where is the pattern information available?> > - Tag instructions as top-of-loop, with nesting information (we use > > this > > to do some static analysis on asm files). > > What part of the code generator would identify this? It seems that > the asmprinter could do this, but it is less obvious than the former > ones. It also seems that this is really a basic block property, not > an instruction property.Yes, we tag basic blocks. That patch is coming later. I added a pass that explicitly examines loop information and adds the appropriate comments. So this could be synthesized on-the-fly, I think.> If these are the planned uses of the comments, it would be nice try to > not add a per-machine-instr list of comments. Instead, the asmprinter > could synthesize the list as it processes each instruction. This > makes the list of comments transient instead of persistent in the > machineinstrs. Does that sound reasonable to you?I still don't know how to synthesize tblgen pattern information in the asmprinter. Except line numbers, which we need today. Maybe all of the other current stuff can be synthesized. I don't know what might come down the road, though. What's the plan for meta-information? Could comments go there when it's ready? Would it be ok to add these for now and remove them as other mechanisms (DebugLoc, meta-information) come on-line? -Dave
On Jul 13, 2009, at 10:02 AM, David Greene wrote:>>> - Tag instructons with source line information (customers really >>> want this). >> >> Right, that would be nice. This should be synthesizable from the >> DebugLoc on the instruction in the asm printer, no need to >> redundantly >> encode it into the comment field. > > Except the DebugLoc stuff isn't all there yet AFAIK. We've been > using the > comment mechanism for over a year. I agree we should move to > synthesizing > it from DebugLoc when it's ready. > > We're not going to submit our line number stuff anyway (it's too > much of a > hack) but we would like the comment infrastructure to be there.DebugLoc is there. The transition isn't complete at the LLVM IR level, but it is at the MachineInstr level AFAIK.>>> - Tag instructions as register spills or reloads. >> >> I'm not sure what this means exactly, but would it be sufficient for >> the asmprinter to use isLoadFromStackSlot and print this if the FI is >> a spill slot? > > Maybe. I'm not sure what information is available here. The other > thing > this code does is tag it as a vector or scalar spill/reload. > Synthesizing > that might be trickier as you have to take the opcode into account. > It's a > lot of work, at the very least.Vector vs scalar should also be pretty simple, just look at the reg class and the VT's involved. It should all be target independent, probably less than a dozen lines. After the conversion to formatted_raw_ostream, we can discuss the best way to do this. It would be helpful if you gave an example of the output you wanted.>>> - Tag instructions with an ID of the tblgen pattern that generated >>> them. This >>> is super useful for debugging. >> >> this would also be really nice :). This can be generated by the asm >> printer as well. > > How so? Where is the pattern information available?You just want the name of the enum? This is the name field of the TargetInstrDesc. If -print-machineinstrs has it, asmprinter can just as easily :).>>> - Tag instructions as top-of-loop, with nesting information (we use >>> this >>> to do some static analysis on asm files). >> >> What part of the code generator would identify this? It seems that >> the asmprinter could do this, but it is less obvious than the former >> ones. It also seems that this is really a basic block property, not >> an instruction property. > > Yes, we tag basic blocks. That patch is coming later. I added a pass > that explicitly examines loop information and adds the appropriate > comments. > So this could be synthesized on-the-fly, I think.Ok, my question is: "why in a separate pass instead of in the asmprinter"? The idea is that comments inherently have no semantic value, and they are only useful for the asmprinter backend. To reduce complexity and compile time cost, it makes sense to do this stuff in the asmprinter if feasible.>> If these are the planned uses of the comments, it would be nice try >> to >> not add a per-machine-instr list of comments. Instead, the >> asmprinter >> could synthesize the list as it processes each instruction. This >> makes the list of comments transient instead of persistent in the >> machineinstrs. Does that sound reasonable to you? > > I still don't know how to synthesize tblgen pattern information in the > asmprinter. Except line numbers, which we need today. Maybe all of > the other > current stuff can be synthesized. I don't know what might come down > the road, > though.I don't know what you mean by "pattern information". We can obviously enhance tblgen to include any information that we need, but I don't see how the asmprinter wouldn't know something that an earlier pass would. Again, it would be helpful if you included example output to show what you're going for.> What's the plan for meta-information? Could comments go there when > it's > ready? Would it be ok to add these for now and remove them as other > mechanisms (DebugLoc, meta-information) come on-line?Which meta-information, MDNode and friends? These won't really be per- machine-instruction, so that won't help. There is a possibility that we can add MD operands to machineinstrs, which might be sufficient for your needs, but again it is unclear why you want to do any of this stuff earlier than asmprinter. -Chris
On Monday 13 July 2009 13:13, Chris Lattner wrote:> > We're not going to submit our line number stuff anyway (it's too > > much of a > > hack) but we would like the comment infrastructure to be there. > > DebugLoc is there. The transition isn't complete at the LLVM IR > level, but it is at the MachineInstr level AFAIK.Well, the IR level is pretty important to us since we convert our code into LLVM IR and add source line comments at the same time.> Vector vs scalar should also be pretty simple, just look at the reg > class and the VT's involved. It should all be target independent, > probably less than a dozen lines. After the conversion to > formatted_raw_ostream, we can discuss the best way to do this. It > would be helpful if you gave an example of the output you wanted.Ok, this might work.> I don't know what you mean by "pattern information". We can obviously > enhance tblgen to include any information that we need, but I don't > see how the asmprinter wouldn't know something that an earlier pass > would. Again, it would be helpful if you included example output to > show what you're going for.Ok, here's an example: movl (%rsi), %ecx # LLVM Instruction: volatile store i32* %ksize, i32** %ksize3 ; [oox.4 : sln.5] # [result] Pattern 1367 I then hacked tblgen to output this in the generated isel file: // Pattern 1367: (ld:i32 addr:iPTR:$src)<<P:Predicate_load>> // Emits: (MOV32rm:i32 addr:iPTR:$src) PatternID = 1367; // Pattern complexity = 19 cost = 1 size = 3 SDValue CPTmp01; SDValue CPTmp11; SDValue CPTmp21; SDValue CPTmp31; if (SelectAddr(N, N1, CPTmp01, CPTmp11, CPTmp21, CPTmp31)) { CurDAG->setSubgraphColor(N.getNode(), "red"); SDNode *Result = Emit_125(N, X86::MOV32rm, MVT::i32, CPTmp01, CPTmp11, CPTmp21, CPTmp31, PatternID); if(Result) { CurDAG->setSubgraphColor(Result, "yellow"); CurDAG->setSubgraphColor(Result, "black"); } return Result; } } So it's really easy to set a breakpoint in Emit_125 to look for the pattern number in question. I forgot about the LLVM instruction information. That's something that can't be synthesized by the asmprinter. Again, we only emit some of this with special debug flags so we don't carry the original IR around in comments all the time. :)> > What's the plan for meta-information? Could comments go there when > > it's > > ready? Would it be ok to add these for now and remove them as other > > mechanisms (DebugLoc, meta-information) come on-line? > > Which meta-information, MDNode and friends?I don't know, I've just heard the term from time to time. Sounds like it's not appropriate for this, though. -Dave