[llvm-commits] [llvm] r164173 - in /llvm/trunk: include/llvm/Transforms/Utils/IntegerDivision.h lib/Transforms/Utils/CMakeLists.txt lib/Transforms/Utils/IntegerDivision.cpp

[Alex Rosenberg]

On Sep 18, 2012, at 3:35 PM, Eli Friedman wrote:

> On Tue, Sep 18, 2012 at 3:02 PM, Michael Ilseman <milseman at apple.com> wrote:
>> Author: milseman
>> Date: Tue Sep 18 17:02:40 2012
>> New Revision: 164173
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=164173&view=rev
>> Log:
>> New utility for expanding integer division for targets that don't support it.
>> 
>> Implementation derived from compiler-rt's implementation of signed and unsigned integer division.
> 
> This commit needs more explanation, given that you haven't added any
> uses of expandDivision; how do you expect this to be used?
> 
> Why is this specialized for 32-bit division?
> 
> There are a lot of missed optimization opportunities here, e.g.
> dividing by a constant; have you considered anything in this area?

Let me pile on a few questions too:

* Why do this expansion in IR and not improve the SelectionDAG's Legalize/Expand code?
* Why expand inline and not save code space by using glue like compiler-rt?
* When will this transform run relative to others? What interaction with other passes is presumed?
* Do you have more large expansions like this coming? Is there any common functionality that should be pulled out?
* Should we change compiler-rt to rely on this expansion?

+------------------------------------------------------------+
| Alexander M. Rosenberg        <mailto:alexr at leftfield.org> |
| Nobody cares what I say, so no disclaimer appears here.    |