[llvm-dev] [RFC] Improving integer divide optimization (related to D12082)

[Mehdi Amini via llvm-dev]

> On Aug 20, 2015, at 9:46 AM, Steve King <steve at metrokings.com> wrote:
> 
> On Wed, Aug 19, 2015 at 10:58 PM, Mehdi Amini <mehdi.amini at apple.com> wrote:
>> 
>> Isn’t the problem the fact that the patch makes it harder for a target to
>> get the generic code to reach its custom hook?
>> Now the "cheap pow2 sdiv” is merged with the generic “cheap div” you can’t
>> distinguish anymore.
>> 
> 
> Yes and also the issue of needing more information to make a smart
> isIntDivCheap() decision.
> 
> In visitSDIV(), checking looks like this when the denominator is a power of 2.
> 
>    if (TLI.isIntDivCheap(N->getValueType(0), MinSize))
>      return SDValue();
> 
>    // Target-specific implementation of sdiv x, pow2.
>    if (SDValue Res = BuildSDIVPow2(N))
>      return Res;
> 
> How about this for isIntDivCheap?
> 1) pass Function* to allow the target to decide for itself which
> attributes matter, and

If you want the attributes, I think you should pass the attributes and not the whole Function.

I’m not sure why MinSize does not trigger with -Os by default, Michael is it intended?

> 2) pass a boolean indicating a power-of-2 denominator.
> 
>    const Function* Fn = DAG.getMachineFunction().getFunction();
>    if (TLI.isIntDivCheap(N->getValueType(0), Fn, true))
>      return SDValue();
> 
>    // Target-specific implementation of sdiv x, pow2.
>    if (SDValue Res = BuildSDIVPow2(N))
>      return Res;

Did you consider what I suggested, i.e. inverting the order of the two tests?

Thinking about it more, I even think than removing the first test entirely would even be nicer here.
The default implementation for BuildSDIVPow2() would be:

virtual SDValue BuildSDIVPow2(SDValue N) {
   bool MinSize = …. // choose a default, target can override anyway
   if (TLI.isIntDivCheap(N->getValueType(0), MinSize))
     return N;
   return SDValue();  
}

I believe this would preserve the current default behavior and leave targets the ability to override as much as they want.

— 
Mehdi