[llvm-commits] [llvm] r156804 - /llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp

[Duncan Sands]

Hi Stepan,

> Well. Nobody was reviewed this changes, since I commited it for post-commit
> review. I was sure that it will work fine, since I learned how
> SelectionDAGBuilder::handleJTSwitchCase works and was sure in my patch. We just
> here we need to produce jump table like this:

...

> Relative to
>  > if (Low.ule(TEI)&& TEI.ule(High))
>
> Probably it is a not best way to check whether TEI belongs to range, but I don't
> think that itsa point of bug.
>
> Probably replacement of comparison from signed to unsigned brakes it with someway.

did you notice this routine in SelectionDAGBuilder.h:

   struct CaseCmp {
     bool operator()(const Case &C1, const Case &C2) {
       assert(isa<ConstantInt>(C1.Low) && isa<ConstantInt>(C2.High));
       const ConstantInt* CI1 = cast<const ConstantInt>(C1.Low);
       const ConstantInt* CI2 = cast<const ConstantInt>(C2.High);
       return CI1->getValue().slt(CI2->getValue());
     }

If it's not being used any more then you should delete it.  If it is being used
then you need to change slt to ult.  However I think it would be much better to
set things up so that there is no notion of signed or unsigned, just intervals
of values that map to the same destination [*].

Ciao, Duncan.

[*] Such intervals possibly wrapping around when considered as signed or
unsigned numbers, but if you set things about carefully this won't matter
in any way.