On Mar 3, 2007, at 4:11 PM, Anton Korobeynikov wrote:
Hello, Everyone.
Please find updated LowerSwitch patch (synced with latest APInt-
related
changes).
Very interesting! Can you give an example with before/after code?
Please add this as a testcase as well. Some code comments:
Meta-comment: this should *really* be merged into SelectionDAGISel,
as nothing uses LowerSwitch any more.
+ struct Case {
+ Constant* Low;
+ Constant* High;
+ BasicBlock* BB;
+
+ Case(Constant* _Low = NULL, Constant* _High = NULL,
+ BasicBlock* _BB = NULL):
+ Low(_Low), High(_High), BB(_BB) { }
+ };
+
+ typedef std::vector<Case> CaseVector;
+ typedef std::vector<Case>::iterator CaseItr;
Please rename Case to "CaseRange".
struct CaseCmp {
bool operator () (const LowerSwitch::Case& C1,
const LowerSwitch::Case& C2) {
- const ConstantInt* CI1 = cast<const ConstantInt>(C1.first);
- const ConstantInt* CI2 = cast<const ConstantInt>(C2.first);
- return CI1->getValue().ult(CI2->getValue());
+ const ConstantInt* CI1 = cast<const ConstantInt>(C1.Low);
+ const ConstantInt* CI2 = cast<const ConstantInt>(C2.High);
+ return CI1->getValue().slt(CI2->getValue());
}
I'm not sure that this is safe. It seems that other parts of the
code expect them to be sorted in ult, not slt, order. For example,
this code does:
ICmpInst* Comp = new ICmpInst(ICmpInst::ICMP_ULT, Val, Pivot.Low,
"Pivot");
+unsigned LowerSwitch::Clusterify(CaseVector& Cases, SwitchInst *SI)
+{
Please add a comment, indicating what this does. The "{" goes on the
same line as the proto.
+ std::list<Case> tmpCases;
std::list is really inefficient, please use a vector and drop the
#include. Once you do this, you can eliminate tmpCases, and just
'clusterify' into 'Cases' directly.
+
+ if ((nextValue-currentValue==1) && (currentBB == nextBB)) {
+ I->High = J->High;
+ tmpCases.erase(J++);
+ } else {
+ I = J++;
+ }
Please add a comment indicating what this is doing. Something like
"If the two neighboring cases go to the same destination, merge them
into a single case".
+ Cases.clear();
This shouldn't be needed.
For things like this:
+ if (I->Low != I->High)
+ // A range counts double, since it requires two compares.
+ ++numCmps;
Please format it like:
+ // A range counts double, since it requires two compares.
+ if (I->Low != I->High)
+ ++numCmps;
or put {}'s around the if block.
Please resend an updated version,
-Chris
_______________________________________________
llvm-commits mailing list
llvm-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
