hi, currently, the DAGCombiner unconditionally converts (DAGCombiner::visit(U|S)REM) expressions of the form X % C for constants C into X-X/C*C. this makes sense in certain cases where the div/mul logic will simplify X/C*X but is counterproductive in general, especially if the multiply is expensive. also, this doesn't allow targets to custom lower rem operations. shouldn't this transformation be left for the legalizer (where it's implemented anyway)? regards, - dietmar
On Thu, 14 Jun 2007, Dietmar Ebner wrote:> currently, the DAGCombiner unconditionally converts > (DAGCombiner::visit(U|S)REM) expressions of the form X % C for constants > C into X-X/C*C. this makes sense in certain cases where the div/mul > logic will simplify X/C*X but is counterproductive in general, > especially if the multiply is expensive. also, this doesn't allow > targets to custom lower rem operations. shouldn't this transformation be > left for the legalizer (where it's implemented anyway)?Yes, the legalizer should do this transformation. -Chris -- http://nondot.org/sabre/ http://llvm.org/
On Mon, 18 Jun 2007, Chris Lattner wrote:> On Thu, 14 Jun 2007, Dietmar Ebner wrote: >> currently, the DAGCombiner unconditionally converts >> (DAGCombiner::visit(U|S)REM) expressions of the form X % C for constants >> C into X-X/C*C. this makes sense in certain cases where the div/mul >> logic will simplify X/C*X but is counterproductive in general, >> especially if the multiply is expensive. also, this doesn't allow >> targets to custom lower rem operations. shouldn't this transformation be >> left for the legalizer (where it's implemented anyway)? > > Yes, the legalizer should do this transformation.Actually, perhaps not. The reason that dag combine does this is that it knows tricky ways to turn division by a constant into multiplication by a constant (plus magic). Doing this xform allows the rem case to take advantage of this. That said, I'm sure there are some targets and some cases where this is counter productive. Are you hitting one? -Chris -- http://nondot.org/sabre/ http://llvm.org/