Richard Biener <rguent...@suse.de> writes:
> On Mon, 9 Nov 2015, Prathamesh Kulkarni wrote:
>> c) Gating the divmod transform -
>> I tried gating it on checks for optab_handlers on div and mod, however
>> this doesn't enable transform for arm cortex-a9
>> anymore (cortex-a9 doesn't have hardware instructions for integer div
>> and mod).
>> IIUC for cortex-a9,
>> optab_handler (sdivmod_optab, SImode) returns CODE_FOR_nothing because
>> HAVE_divsi3 is 0.
>> However optab_handler (smod_optab, SImode) matches since optab_handler
>> only checks for existence of pattern
>> (and not whether the pattern gets matched).
>> I suppose we should enable the transform only if the divmod, div, and
>> mod pattern do not match rather than checking
>> if the patterns exist via optab_handler ? For a general x % y, modsi3
>> would fail to match but optab_handler(smod_optab, SImode ) still
>> says it's matched.
>
> Ah, of course. Querying for an optab handler is just a cheap
> guesstimate... Not sure how to circumvent this best (sub-target
> enablement of patterns). RTL expansion just goes ahead (of course)
> and sees if expansion eventually fails. Richard?
Yeah, FAILs in expanders are kind of awkward these days. The ARM
modsi3 could be a bit more helpful by having a predicate that only
accepts power-of-2 integers instead of any const_int_operand,
which would at least avoid having to generate insns.
I did wonder once whether the generators should provide a way of
automatically fusing separate .md constructs into a single optab.
That might avoid more complicated FAILs and could be used to
automatically generate information for the tree level.
>> Should we define a new target hook combine_divmod, which returns true
>> if transforming to divmod is desirable for that
>> target ?
>> The default definition could be:
>> bool default_combine_divmod (enum machine_mode mode, tree op1, tree op2)
>> {
>> // check for optab_handlers for div/mod/divmod and libfunc for divmod
>> }
>>
>> And for arm, it could be over-ridden to return false if op2 is
>> constant and <= 0 or power of 2.
>> I am not really sure if this is a good idea since I am replicating
>> information from modsi3 pattern.
>> Any change to the pattern may require corresponding change to the hook :/
>
> Yeah, I don't think that is desirable. Ideally we'd have a way
> to query HAVE_* for CODE_FOR_* which would mean target-insns.def
> support for all div/mod/divmod patterns(?) and queries...
>
> Not sure if what would be enough though.
This kind of stuff should really go through optabs rather than
target-insns.def. target-insns.def is more for cases where there's
no distinction between operand modes.
> Note that the divmod check is equally flawed.
>
> I think with the above I'd enable the transform when
>
> + if (optab_handler (divmod_optab, mode) != CODE_FOR_nothing
> + || (optab_libfunc (divmod_optab, mode) != NULL_RTX
> && optab_handler ([su]div_optab, mode) == CODE_FOR_nothing))
> + return false;
>
> so we either will have a divmod instruction (hopefully not sub-target
> disabled for us) or a libfunc for divmod and for sure no HW divide
> instruction (HW mod can be emulated by HW divide but not the other
> way around).
Sounds good to me FWIW.
Thanks,
Richard
