[llvm-commits] llvm-gcc: emit switch cases with a wide range as a conditional branch

[Reid Spencer]

On Wed, 2007-03-14 at 17:40 +0100, Duncan Sands wrote:
> 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? 

Currently, ConstantInt is implemented in terms of APInt. So, if you
subtract two ConstantInt (via ConstantExpr), you are in essence just
doing an APInt subtraction. However, having just looked at the constant
folding code again, here's what happens:

     1. Call to ConstantExpr::get which calls ConstantExpr::getTy which
        calls ConstantFoldBinaryInstruction.
     2. 5 if statements and a switch in ConstantFoldBinaryInstruction
     3. Extraction of APInt values from the ConstantInt operands to the
        ConstantExpr object.
     4. APInt subtraction.
     5. Construction of a new ConstantInt which involves at least (a)
        construction of an IntegerMapKeyType and (b) std::map lookup to
        see if the value already exists; and, if the value doesn't
        exist, will also involve (c) instantiation of an integer value
        representation object and (d) insertion into two maps (one
        forward, one inverse)

I think this is Chris' point. You could replace all of that with just
"APInt subtraction". #5 can be really expensive because of the map
traversals and insertions, especially in programs with lots of constant
values.

>  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?  

It probably is. 

> 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.

Probably :)

> 
> > +    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.

Since you are working with numbers of bits here and not with values
requiring that number of bits, the range of values possible will always
fit in a uint32_t. Is it possible just to use that instead of a
ConstantInt?  uint32_t Range = X - Y is way cheaper than any of the
foregoing :)

> 
> 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.

FWIW, I think 2*HOST_BITS_PER_WIDE_INT is correct. GCC uses that all
over the place because a CONSTANT_INT node in gcc is a structure
containing two HOST_WIDE_INT integer values (lo and hi). You want to
stay within that range.

FYI: I'm contemplating making llvm-gcc represent CONSTANT_INT nodes with
APInt so that it can handle integer constants of any bit width. This is
a requirement for the work I'm doing for AutoESL.

> 
> Ciao,
> 
> Duncan.
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits