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/
hi, thanks for your answer. On Mon, 2007-06-18 at 23:25 -0700, Chris Lattner wrote:> 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?yes. i'm compiling for a target without native div/rem instructions. when compiling expressions like (a % 3), the dagcombiner turns them into (a-(a/3)*3) which is not simplified later on. this is quite expensive since the multiply is not pipelined and the div requires a libcall while the same could have been achieved with a (S|U)REM_I32 libcall. the problem is that the involved "magic" int TargetLowering has certain requirements such as MULHU support, which is not available on my architecture. i guess the best way is to check this conditions beforehand in the DAGCombiner, right? cheers, - dietmar
On Tue, 19 Jun 2007, Dietmar Ebner wrote:>> 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?> yes. i'm compiling for a target without native div/rem instructions. > when compiling expressions like (a % 3), the dagcombiner turns them into > (a-(a/3)*3) which is not simplified later on. this is quite expensive > since the multiply is not pipelined and the div requires a libcall while > the same could have been achieved with a (S|U)REM_I32 libcall. > > the problem is that the involved "magic" int TargetLowering has certain > requirements such as MULHU support, which is not available on my > architecture. i guess the best way is to check this conditions beforehand > in the DAGCombiner, right?Ah, right. The problem is that you don't have div *or* MULHU/MULHS. I think you're right: please conditionalize the xform on the availability of these instructions. Having either MULH* or DIV should allow the xform. -Chris -- http://nondot.org/sabre/ http://llvm.org/