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

[Evan Cheng]

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?

    // Get information about the SDNode for the operator.
     const SDNodeInfo &OpInfo = CDP.getSDNodeInfo(N->getOperator());

     // Notice properties of the node.
     if (OpInfo.hasProperty(SDNPMayStore)) mayStore = true;
     if (OpInfo.hasProperty(SDNPMayLoad)) mayLoad = true;


+  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?

Thanks,

Evan

On Jan 31, 2008, at 6:08 PM, Dan Gohman wrote:

> Attached is an updated version of this change (46556 and 46585)
> with what I believe is a fix for the static initializer problem,
> but I don't have access to a system where this problem causes
> failures. Could I ask someone who does to try it out?
>
> Thanks,
>
> Dan
>
> -- 
> Dan Gohman, Cray Inc.
> <new.patch>_______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits