[PATCH] TableGen: Allow AddedComplexity values to be negative
Hal Finkel
hfinkel at anl.gov
Tue Jul 8 09:02:12 PDT 2014
----- Original Message -----
> From: "Tom Stellard" <tom at stellard.net>
> To: "Hal Finkel" <hfinkel at anl.gov>
> Cc: "Tom Stellard" <thomas.stellard at amd.com>, llvm-commits at cs.uiuc.edu
> Sent: Thursday, July 3, 2014 10:07:23 AM
> Subject: Re: [PATCH] TableGen: Allow AddedComplexity values to be negative
>
> On Mon, Jun 30, 2014 at 04:29:54PM -0500, Hal Finkel wrote:
> > Tom,
> >
> > I suppose this is fine. I'd prefer a different solution altogether,
> > where we actually named the pattern or instruction that we'd like
> > to be ordered before or after and let TableGen compute some
> > appropriate numbering. What do you think?
> >
>
> Are you proposing a system for ranking patterns?
Yes.
To be clear, this change LGTM.
But I wanted to use this opportunity to get your opinion of whether a better overall scheme was available. Personally, I find the "magic number" aspect of the current scheme problematic. Understanding the overall ordering of the patterns is difficult because the added complexity numbers are often spread among multiple files, and they also interact in unseen ways with the internally-generated pattern complexities. It seems to me that, often, we use these added complexities when we have a specific ordering in mind, and expressing that ordering explicitly seems like a better solution. What do you think?
Thanks again,
Hal
>
> -Tom
>
> > -Hal
> >
> > ----- Original Message -----
> > > From: "Tom Stellard" <thomas.stellard at amd.com>
> > > To: llvm-commits at cs.uiuc.edu
> > > Cc: "Tom Stellard" <thomas.stellard at amd.com>
> > > Sent: Monday, June 30, 2014 4:11:31 PM
> > > Subject: [PATCH] TableGen: Allow AddedComplexity values to be
> > > negative
> > >
> > > This is useful for cases when stand-alone patterns are preferred
> > > to
> > > the
> > > patterns included in the instruction definitions. Instead of
> > > requiring
> > > that stand-alone patterns set a larger AddedComplexity value,
> > > which
> > > can be confusing to new developers, the allows us to reduce the
> > > complexity of the included patterns to achieve the same result.
> > > ---
> > > test/TableGen/NegativeAddedComplexity.ll | 41
> > > ++++++++++++++++++++++++++++++++
> > > utils/TableGen/CodeGenDAGPatterns.cpp | 2 +-
> > > utils/TableGen/CodeGenDAGPatterns.h | 8 +++----
> > > utils/TableGen/DAGISelEmitter.cpp | 4 ++--
> > > 4 files changed, 48 insertions(+), 7 deletions(-)
> > > create mode 100644 test/TableGen/NegativeAddedComplexity.ll
> > >
> > > diff --git a/test/TableGen/NegativeAddedComplexity.ll
> > > b/test/TableGen/NegativeAddedComplexity.ll
> > > new file mode 100644
> > > index 0000000..54c52ab
> > > --- /dev/null
> > > +++ b/test/TableGen/NegativeAddedComplexity.ll
> > > @@ -0,0 +1,41 @@
> > > +// RUN: llvm-tblgen -I../../include -gen-dag-isel %s | FileCheck
> > > %s
> > > +// XFAIL: vg_leak
> > > +
> > > +include "llvm/Target/Target.td"
> > > +
> > > +// Make sure the higher complexity pattern comes first
> > > +// CHECK: TARGET_VAL(::ADD0)
> > > +// CHECK: Complexity = {{[^-]}}
> > > +// Make sure the ADD1 pattern has a negative complexity
> > > +// CHECK: TARGET_VAL(::ADD1)
> > > +// CHECK: Complexity = -{{[0-9]+}}
> > > +
> > > +def TestRC : RegisterClass<"TEST", [i32], 32, (add)>;
> > > +
> > > +def TestInstrInfo : InstrInfo;
> > > +
> > > +def Test : Target {
> > > + let InstructionSet = TestInstrInfo;
> > > +}
> > > +
> > > +def ADD0 : Instruction {
> > > + let OutOperandList = (outs TestRC:$dst);
> > > + let InOperandList = (ins TestRC:$src0, TestRC:$src1);
> > > +}
> > > +
> > > +def ADD1 : Instruction {
> > > + let OutOperandList = (outs TestRC:$dst);
> > > + let InOperandList = (ins TestRC:$src0, TestRC:$src1);
> > > +}
> > > +
> > > +def : Pat <
> > > + (add i32:$src0, i32:$src1),
> > > + (ADD1 $src0, $src1)
> > > +> {
> > > + let AddedComplexity = -1000;
> > > +}
> > > +
> > > +def : Pat <
> > > + (add i32:$src0, i32:$src1),
> > > + (ADD0 $src0, $src1)
> > > +>;
> > > diff --git a/utils/TableGen/CodeGenDAGPatterns.cpp
> > > b/utils/TableGen/CodeGenDAGPatterns.cpp
> > > index 00bc9a5..fa94fd3 100644
> > > --- a/utils/TableGen/CodeGenDAGPatterns.cpp
> > > +++ b/utils/TableGen/CodeGenDAGPatterns.cpp
> > > @@ -771,7 +771,7 @@ static unsigned getPatternSize(const
> > > TreePatternNode *P,
> > >
> > > /// Compute the complexity metric for the input pattern. This
> > > roughly
> > > /// corresponds to the number of nodes that are covered.
> > > -unsigned PatternToMatch::
> > > +int PatternToMatch::
> > > getPatternComplexity(const CodeGenDAGPatterns &CGP) const {
> > > return getPatternSize(getSrcPattern(), CGP) +
> > > getAddedComplexity();
> > > }
> > > diff --git a/utils/TableGen/CodeGenDAGPatterns.h
> > > b/utils/TableGen/CodeGenDAGPatterns.h
> > > index fb30cdd..ef6c787 100644
> > > --- a/utils/TableGen/CodeGenDAGPatterns.h
> > > +++ b/utils/TableGen/CodeGenDAGPatterns.h
> > > @@ -667,7 +667,7 @@ public:
> > > PatternToMatch(Record *srcrecord, ListInit *preds,
> > > TreePatternNode *src, TreePatternNode *dst,
> > > const std::vector<Record*> &dstregs,
> > > - unsigned complexity, unsigned uid)
> > > + int complexity, unsigned uid)
> > > : SrcRecord(srcrecord), Predicates(preds), SrcPattern(src),
> > > DstPattern(dst),
> > > Dstregs(dstregs), AddedComplexity(complexity), ID(uid) {}
> > >
> > > @@ -676,7 +676,7 @@ public:
> > > TreePatternNode *SrcPattern; // Source pattern to match.
> > > TreePatternNode *DstPattern; // Resulting pattern.
> > > std::vector<Record*> Dstregs; // Physical register defs being
> > > matched.
> > > - unsigned AddedComplexity; // Add to matching pattern
> > > complexity.
> > > + int AddedComplexity; // Add to matching pattern
> > > complexity.
> > > unsigned ID; // Unique ID for the record.
> > >
> > > Record *getSrcRecord() const { return SrcRecord; }
> > > @@ -684,13 +684,13 @@ public:
> > > TreePatternNode *getSrcPattern() const { return SrcPattern; }
> > > TreePatternNode *getDstPattern() const { return DstPattern; }
> > > const std::vector<Record*> &getDstRegs() const { return
> > > Dstregs; }
> > > - unsigned getAddedComplexity() const { return
> > > AddedComplexity; }
> > > + int getAddedComplexity() const { return
> > > AddedComplexity; }
> > >
> > > std::string getPredicateCheck() const;
> > >
> > > /// Compute the complexity metric for the input pattern. This
> > > roughly
> > > /// corresponds to the number of nodes that are covered.
> > > - unsigned getPatternComplexity(const CodeGenDAGPatterns &CGP)
> > > const;
> > > + int getPatternComplexity(const CodeGenDAGPatterns &CGP) const;
> > > };
> > >
> > > class CodeGenDAGPatterns {
> > > diff --git a/utils/TableGen/DAGISelEmitter.cpp
> > > b/utils/TableGen/DAGISelEmitter.cpp
> > > index 82682cd..e2e6ab1 100644
> > > --- a/utils/TableGen/DAGISelEmitter.cpp
> > > +++ b/utils/TableGen/DAGISelEmitter.cpp
> > > @@ -94,8 +94,8 @@ struct PatternSortingPredicate {
> > > // Otherwise, if the patterns might both match, sort based
> > > on
> > > complexity,
> > > // which means that we prefer to match patterns that cover
> > > more
> > > nodes in the
> > > // input over nodes that cover fewer.
> > > - unsigned LHSSize = LHS->getPatternComplexity(CGP);
> > > - unsigned RHSSize = RHS->getPatternComplexity(CGP);
> > > + int LHSSize = LHS->getPatternComplexity(CGP);
> > > + int RHSSize = RHS->getPatternComplexity(CGP);
> > > if (LHSSize > RHSSize) return true; // LHS -> bigger ->
> > > less
> > > cost
> > > if (LHSSize < RHSSize) return false;
> > >
> > > --
> > > 1.8.1.5
> > >
> > > _______________________________________________
> > > llvm-commits mailing list
> > > llvm-commits at cs.uiuc.edu
> > > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> > >
> >
> > --
> > Hal Finkel
> > Assistant Computational Scientist
> > Leadership Computing Facility
> > Argonne National Laboratory
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
--
Hal Finkel
Assistant Computational Scientist
Leadership Computing Facility
Argonne National Laboratory
More information about the llvm-commits
mailing list