This is an archive of the discontinued LLVM Phabricator instance.

[TTI] Add generic cost model for fixed point smul/umul
ClosedPublic

Authored by RKSimon on Feb 7 2019, 1:31 PM.

Details

Summary

Based on an IR equivalent of target lowering's generic expansion - target specific costs will typically be lower (IR doesn't have a good mull/mulh equivalent) but we need a baseline.

An alternative would be to assume the mull/mulh cost is the same as 2 * mul - what do people think?

Diff Detail

Repository
rL LLVM

Event Timeline

RKSimon created this revision.Feb 7 2019, 1:31 PM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 7 2019, 1:31 PM
nikic added a comment.Feb 12 2019, 1:20 PM

An alternative would be to assume the mull/mulh cost is the same as 2 * mul - what do people think?

Would it be possible to add a separate method for getting the widening multiplication cost, defaulting to your current implementation? It would come in handy for MULO cost modeling as well. And probably more likely to receive a good target-specific implementation than the full smul.fix intrinsics.

An alternative would be to assume the mull/mulh cost is the same as 2 * mul - what do people think?

Would it be possible to add a separate method for getting the widening multiplication cost, defaulting to your current implementation? It would come in handy for MULO cost modeling as well. And probably more likely to receive a good target-specific implementation than the full smul.fix intrinsics.

Sorry, I missed your comment! The existing method to do this is to pass Value arguments to getArithmeticInstrCost, there are already some targets that use this to detect extend+add/mul style cases. The problem we have here is that we don't really want to create Values on the fly for the analysis, and adding a Type equivalent version would be a little messy and cause duplication (we already suffer with this with the 2 getIntrinsicInstrCost variants).

My hope would be that for targets where this matters they will implement an override.

nikic added a comment.Feb 24 2019, 6:29 AM

The existing method to do this is to pass Value arguments to getArithmeticInstrCost, there are already some targets that use this to detect extend+add/mul style cases. The problem we have here is that we don't really want to create Values on the fly for the analysis, and adding a Type equivalent version would be a little messy and cause duplication (we already suffer with this with the 2 getIntrinsicInstrCost variants).

Another possibility might be to add an OperandValueProperties flag. Right now it's used only for OP_PowerOf2, but we could also have something like OP_ZExtHalf and OP_SExtHalf or similar.

nikic accepted this revision.Feb 24 2019, 6:49 AM

In any case, the implementation seems like a reasonable baseline to start with, we can try to make the mul cost modelling more accurate as a followup.

This revision is now accepted and ready to land.Feb 24 2019, 6:49 AM
This revision was automatically updated to reflect the committed changes.