[llvm-commits] [llvm] r58278 - in /llvm/trunk: include/llvm/CodeGen/DAGISelHeader.h lib/Target/ARM/ARMISelDAGToDAG.cpp lib/Target/Alpha/AlphaISelDAGToDAG.cpp lib/Target/CellSPU/SPUISelDAGToDAG.cpp lib/Target/IA64/IA64ISelDAGToDAG.cpp lib/Target/Mips/MipsISelDAGToDAG.cpp lib/Target/PIC16/PIC16ISelDAGToDAG.cpp lib/Target/PowerPC/PPCISelDAGToDAG.cpp lib/Target/Sparc/SparcISelDAGToDAG.cpp lib/Target/X86/X86ISelDAGToDAG.cpp utils/TableGen/DAGISelEmitter.cpp
Dan Gohman
gohman at apple.com
Mon Oct 27 18:00:31 PDT 2008
On Oct 27, 2008, at 2:56 PM, David Greene wrote:
> Author: greened
> Date: Mon Oct 27 16:56:29 2008
> New Revision: 58278
>
> URL: http://llvm.org/viewvc/llvm-project?rev=58278&view=rev
> Log:
>
> Have TableGen emit setSubgraphColor calls under control of a -gen-
> debug
> flag. Then in a debugger developers can set breakpoints at these
> calls
> to see waht is about to be selected and what the resulting subgraph
> looks like. This really helps when debugging instruction selection.
Cool! Comments below.
>
>
> Modified:
> llvm/trunk/include/llvm/CodeGen/DAGISelHeader.h
> llvm/trunk/lib/Target/ARM/ARMISelDAGToDAG.cpp
> llvm/trunk/lib/Target/Alpha/AlphaISelDAGToDAG.cpp
> llvm/trunk/lib/Target/CellSPU/SPUISelDAGToDAG.cpp
> llvm/trunk/lib/Target/IA64/IA64ISelDAGToDAG.cpp
> llvm/trunk/lib/Target/Mips/MipsISelDAGToDAG.cpp
> llvm/trunk/lib/Target/PIC16/PIC16ISelDAGToDAG.cpp
> llvm/trunk/lib/Target/PowerPC/PPCISelDAGToDAG.cpp
> llvm/trunk/lib/Target/Sparc/SparcISelDAGToDAG.cpp
> llvm/trunk/lib/Target/X86/X86ISelDAGToDAG.cpp
> llvm/trunk/utils/TableGen/DAGISelEmitter.cpp
>
> Modified: llvm/trunk/include/llvm/CodeGen/DAGISelHeader.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/CodeGen/DAGISelHeader.h?rev=58278&r1=58277&r2=58278&view=diff
>
> =
> =
> =
> =
> =
> =
> =
> =
> ======================================================================
> --- llvm/trunk/include/llvm/CodeGen/DAGISelHeader.h (original)
> +++ llvm/trunk/include/llvm/CodeGen/DAGISelHeader.h Mon Oct 27
> 16:56:29 2008
> @@ -153,7 +153,7 @@
>
> /// SelectRoot - Top level entry to DAG instruction selector.
> /// Selects instructions starting at the root of the current DAG.
> -void SelectRoot() {
> +void SelectRoot(SelectionDAG &DAG) {
This DAG parameter shouldn't be necessary. SelectRoot is a member
of SelectionDAGISel, where it can access the CurDAG member.
>
> SelectRootInit();
> unsigned NumBytes = (DAGSize + 7) / 8;
> ISelQueued = new unsigned char[NumBytes];
> @@ -176,14 +176,18 @@
> // Skip already selected nodes.
> if (isSelected(Node->getNodeId()))
> continue;
> + DAG.setSubgraphColor(Node, "red");
I think this line should be guarded by #ifndef NDEBUG, so that
it doesn't slow down release mode. Either that, or perhaps
setSubgraphColor itself should be an empty inline function in
the NDEBUG case.
>
> SDNode *ResNode = Select(SDValue(Node, 0));
> // If node should not be replaced,
> // continue with the next one.
> if (ResNode == Node)
> continue;
> // Replace node.
> - if (ResNode)
> + if (ResNode) {
> + DAG.setSubgraphColor(ResNode, "yellow");
> + DAG.setSubgraphColor(ResNode, "black");
Setting the node to yellow and then immediately to black?
>
> ReplaceUses(Node, ResNode);
> + }
> // If after the replacement this node is not used any more,
> // remove this dead node.
> if (Node->use_empty()) { // Don't delete EntryToken, etc.
> @@ -191,6 +195,7 @@
> CurDAG->RemoveDeadNode(Node, &ISQU);
> UpdateQueue(ISQU);
> }
> + //DAG.setSubgraphColor(Node, "black");
Please decide what to do this code, instead of just
leaving it here commented out.
>
> }
>
> delete[] ISelQueued;
>
> Modified: llvm/trunk/utils/TableGen/DAGISelEmitter.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/utils/TableGen/DAGISelEmitter.cpp?rev=58278&r1=58277&r2=58278&view=diff
>
> =
> =
> =
> =
> =
> =
> =
> =
> ======================================================================
> --- llvm/trunk/utils/TableGen/DAGISelEmitter.cpp (original)
> +++ llvm/trunk/utils/TableGen/DAGISelEmitter.cpp Mon Oct 27 16:56:29
> 2008
> @@ -14,13 +14,21 @@
> #include "DAGISelEmitter.h"
> #include "Record.h"
> #include "llvm/ADT/StringExtras.h"
> +#include "llvm/Support/CommandLine.h"
> #include "llvm/Support/Debug.h"
> #include "llvm/Support/MathExtras.h"
> +#include "llvm/Support/Debug.h"
> #include "llvm/Support/Streams.h"
> #include <algorithm>
> #include <deque>
> using namespace llvm;
>
> +namespace {
> + cl::opt<bool>
> + GenDebug("gen-debug", cl::desc("Generate debug code"),
> + cl::init(false));
> +}
As a tablegen option, this cool functionality is somewhat
out of common view. Would it make sense to turn this functionality
on by default, with the code guarded by #ifndef NDEBUG?
>
> +
> //
> =
> =
> =
> ----------------------------------------------------------------------=
> ==//
> // DAGISelEmitter Helper methods
> //
> @@ -969,6 +977,10 @@
> emitCode("InChains.push_back(" + ChainName + ");");
> emitCode(ChainName + " = CurDAG->getNode(ISD::TokenFactor,
> MVT::Other, "
> "&InChains[0], InChains.size());");
> + if (GenDebug) {
> + emitCode("CurDAG->setSubgraphColor(" + ChainName
> +".getNode(), \"yellow\");");
> + emitCode("CurDAG->setSubgraphColor(" + ChainName
> +".getNode(), \"black\");");
> + }
Again, yellow and then immediately black?
>
> }
>
> // Loop over all of the operands of the instruction pattern,
> emitting code
> @@ -1096,13 +1108,18 @@
> if (II.isSimpleLoad | II.mayLoad | II.mayStore) {
> std::vector<std::string>::const_iterator mi, mie;
> for (mi = LSI.begin(), mie = LSI.end(); mi != mie; ++mi) {
> - emitCode("SDValue LSI_" + *mi + " = "
> + std::string LSIName = "LSI_" + *mi;
> + emitCode("SDValue " + LSIName + " = "
> "CurDAG->getMemOperand(cast<MemSDNode>(" +
> *mi + ")->getMemOperand());");
> + if (GenDebug) {
> + emitCode("CurDAG->setSubgraphColor(" + LSIName
> +".getNode(), \"yellow\");");
> + emitCode("CurDAG->setSubgraphColor(" + LSIName
> +".getNode(), \"black\");");
> + }
> if (IsVariadic)
> - emitCode("Ops" + utostr(OpsNo) + ".push_back(LSI_" +
> *mi + ");");
> + emitCode("Ops" + utostr(OpsNo) + ".push_back(" +
> LSIName + ");");
> else
> - AllOps.push_back("LSI_" + *mi);
> + AllOps.push_back(LSIName);
> }
> }
>
> @@ -1269,6 +1286,18 @@
> }
>
> emitCode(CodePrefix + Code + ");");
> +
> + if (GenDebug) {
> + if (!isRoot) {
> + emitCode("CurDAG->setSubgraphColor(" + NodeName
> +".getNode(), \"yellow\");");
> + emitCode("CurDAG->setSubgraphColor(" + NodeName
> +".getNode(), \"black\");");
> + }
> + else {
> + emitCode("CurDAG->setSubgraphColor(" + NodeName +",
> \"yellow\");");
> + emitCode("CurDAG->setSubgraphColor(" + NodeName +",
> \"black\");");
> + }
> + }
> +
> for (unsigned i = 0, e = After.size(); i != e; ++i)
> emitCode(After[i]);
>
> @@ -1766,8 +1795,19 @@
>
> // Replace the emission code within selection routines with
> calls to the
> // emission functions.
> - CallerCode = "return Emit_" + utostr(EmitFuncNum) +
> CallerCode;
> - GeneratedCode.push_back(std::make_pair(false, CallerCode));
> + if (GenDebug) {
> + GeneratedCode.push_back(std::make_pair(0, "CurDAG-
> >setSubgraphColor(N.getNode(), \"red\");"));
> + }
> + CallerCode = "SDNode *Result = Emit_" + utostr(EmitFuncNum)
> + CallerCode;
> + GeneratedCode.push_back(std::make_pair(3, CallerCode));
> + if (GenDebug) {
> + GeneratedCode.push_back(std::make_pair(0, "if(Result) {"));
> + GeneratedCode.push_back(std::make_pair(0, " CurDAG-
> >setSubgraphColor(Result, \"yellow\");"));
> + GeneratedCode.push_back(std::make_pair(0, " CurDAG-
> >setSubgraphColor(Result, \"black\");"));
> + GeneratedCode.push_back(std::make_pair(0, "}"));
> + //GeneratedCode.push_back(std::make_pair(0, "CurDAG-
> >setSubgraphColor(N.getNode(), \"black\");"));
> + }
> + GeneratedCode.push_back(std::make_pair(0, "return Result;"));
> }
>
> // Print function.
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
More information about the llvm-commits
mailing list