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

[Hal Finkel]

On Tue, 18 Sep 2012 17:39:54 -0700
Evan Cheng <evan.cheng at apple.com> wrote:

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

This is the wrong thread for this, but since you mention it, I'd really
like to see a serious discussion about this sooner rather than later.
We need this for support of several different areas.

 -Hal

> 
> > * 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
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits



-- 
Hal Finkel
Postdoctoral Appointee
Leadership Computing Facility
Argonne National Laboratory