[llvm-commits] [llvm] r46556 - in /llvm/trunk: include/llvm/CodeGen/PseudoSourceValue.h include/llvm/Value.h lib/CodeGen/PseudoSourceValue.cpp

[Dan Gohman]

 > Some comments on the patch:
 > > + > + // Save loads/stores matched by a pattern.
 > + if (!N->isLeaf() && N->getName().empty() &&
 > + ((N->getOperator()->getName() == "ld") ||
 > + (N->getOperator()->getName() == "st") ||
  > + (N->getOperator()->getName() == "ist"))) {
 > + LSI.push_back(RootName); > + }
 > +
 >
 > I am not sure about this. Perhaps it should be similar to
 > what
 > InstrInfoEmitter.cpp is doing?

The MayStore and MayLoad properties are per-instruction; the code  
above needs to know which specific SDNodes in the pattern will be  
represented with StoreSDNode or LoadSDNode.
An alternative to checking for "st" and friends would be to check if  
the node's Opcode field is one of the strings "ISD::STORE" or  
"ISD::LOAD"; I guess that's a little more flexible.
 > + static const char *PSVNames[] = {
 > + "FPRel", > + "SPRel", > + "GPRel",
 > + "TPRel",
 > + "CPRel",
 > + "JTRel"
 > + };
  >
  > I am taking exception to the names. FPRel looks too much like it has
 > something to do with FP register, GPRel looks like it is referring to
 > general purpose register. How about just spill it out? e.g.
 > StackObjRel, FixedStackObjRel, GOTRel, ThreadPtrRel,
 > ConstPoolRel, JumpTabRel?

Sounds reasonable to me. I'll update this before committing.

Dan