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

[Evan Cheng]

On Sep 18, 2012, at 4:52 PM, Alex Rosenberg <alexr at leftfield.org> wrote:

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

Because legalization cannot update CFG.

> * Why expand inline and not save code space by using glue like compiler-rt?

That's still the case with legalization. This is for cases where calls are undesirable.

> * When will this transform run relative to others? What interaction with other passes is presumed?

This commit provides a utility. If / when we have more expansion like this, then someone should introduce a pass.

> * Do you have more large expansions like this coming? Is there any common functionality that should be pulled out?

No immediate plan but it's going to happen. We already have interesting cases where targets want to emit code with versioning. I think we need to fix the problem of exposing target information to LLVM ir pass before we can really design a pass that makes sense. Right now, we stick these fix-ups in codegenprep. That's ok for now, but it's not a good long term solution.

> * Should we change compiler-rt to rely on this expansion?

Not right now. But you can envision that might make sense in the future.

Evan

> 
> +------------------------------------------------------------+
> | Alexander M. Rosenberg        <mailto:alexr at leftfield.org> |
> | Nobody cares what I say, so no disclaimer appears here.    |
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits