Hi Chris, thanks for looking at the patch.
> > In gcc, a switch case is a range of values that branch
> > to a label, for example 1 .. 17 -> label. These are
> > emitted as individual LLVM switch cases: 1 -> label,
> > 2 -> label, ..., 17 -> label. This works well except,
> > for example, when the range is INT_MIN .. 0 -> label,
> > in which case you can say goodbye to all your memory!
> > This patch causes ranges with more than 64 elements
> > (128 on 64 bit machines) to be emitted as explicit "if"
> > statements. For example, the following gcc switch
> > (from the Ada testcase)
>
> The patch looks good with two changes:
>
> + ConstantInt *Range = cast<ConstantInt>(ConstantExpr::getSub(HiC,
> ValC));
>
> Please use APInt's to do the subtraction, instead of constant
> folding. Reid should be able to help you with this.
I don't understand why. If APInt's are much more efficient, then
shouldn't ConstantExpr:getSub be improved to detect this case and
directly use APInts itself? Also, I only do one subtraction and
a comparison - is it worth making things more complicated (OK, only
a little bit more) in such a case? Finally, the existing code does
a loop in which it adds one to the value using ConstantExpr:getAdd;
presumably you would also like this to be changed to use APInts.
> + if (Range->getZExtValue() < 2*HOST_BITS_PER_WIDE_INT) {
>
> This is bad because it means llvm-gcc will produce different code
> based on thevalue of HOST_BITS... Please just say " < 64" or something.
The idea here was that optimizers tend to make different decisions
based on whether the number of cases is <= HOST_BITS_PER_WIDE_INT
or not. I wanted to stay far away from this boundary, and not
second-guess the optimizers, thus the *2. I could use 64, which is
probably OK for 64 bit machines, or 128 in order to be sure not to
be fighting with the optimizers. Any thoughts? If none, I will use
64.
Ciao,
Duncan.
_______________________________________________
llvm-commits mailing list
llvm-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
