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

[Duncan Sands]

Hi Reid, thanks for replying.

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

In most of the switch code #5 cannot be avoided, since the final result
needs to be a ConstantInt.  However Range only needs to be turned into
a ConstantInt if the range is wide, so indeed one instance of #5 can be
avoided in the common case.

> > > +    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 :)

Range is the case range, which can be arbitrarily big, so I'm afraid not.

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

This would make it hard to merge with mainline.

I am testing the following patch.  While I was there, I made it agnostic
as to the signedness of the switch expression and cases (in Ada they can
be unsigned).

Ciao,

Duncan.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: switch.diff
Type: text/x-diff
Size: 3952 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20070314/4c3e2f14/attachment.diff>