DAGCombiner: (S|U)REM

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,

Yes, the legalizer should do this transformation.

-Chris

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

hi,

thanks for your answer.

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

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