Page MenuHomePhabricator

[TTI] Add OperandValueProperties::OP_NegatedPowerOf2 enum
Changes PlannedPublic

Authored by RKSimon on Oct 17 2021, 11:21 AM.

Details

Summary

The div/rem by constant costmodels handle power-of-2 constants, but not negated-power-of-2, despite the backends handling both.

This patch adds the OperandValueProperties::OP_NegatedPowerOf2 enum and wires it for use for basic mul/div/rem cost analysis and SLP handling.

Fixes https://bugs.llvm.org/show_bug.cgi?id=51436

Diff Detail

Event Timeline

RKSimon created this revision.Oct 17 2021, 11:21 AM
RKSimon requested review of this revision.Oct 17 2021, 11:21 AM
Herald added a project: Restricted Project. · View Herald TranscriptOct 17 2021, 11:21 AM
RKSimon added inline comments.Oct 17 2021, 11:21 AM
llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp
1397

@evandro You added pow2 support a while ago but I can't see any test coverage?

xbolva00 added inline comments.
llvm/include/llvm/Analysis/TargetTransformInfo.h
889

Mistake?

RKSimon added inline comments.Oct 17 2021, 11:49 AM
llvm/include/llvm/Analysis/TargetTransformInfo.h
889

yes - not quite sure how that happened.....

RKSimon updated this revision to Diff 380262.Oct 17 2021, 11:58 AM

Actually use the correct diff this time.......

dmgreen added inline comments.Oct 18 2021, 5:06 AM
llvm/lib/Analysis/TargetTransformInfo.cpp
693

This could do with brackets now.

RKSimon added inline comments.Oct 18 2021, 5:58 AM
llvm/lib/Analysis/TargetTransformInfo.cpp
673

I'm starting to think a APInt::isNegatedPowerOf2() wrapper might be useful?

RKSimon updated this revision to Diff 380375.Oct 18 2021, 6:47 AM

Add braces

RKSimon updated this revision to Diff 380685.Oct 19 2021, 7:12 AM
RKSimon edited the summary of this revision. (Show Details)

Rebased - use APInt::isNegatedPowerOf2()

dmgreen added inline comments.Oct 20 2021, 2:01 AM
llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp
1397

It looks like this goes back to https://reviews.llvm.org/D5469, and only included SLP vectorization tests. I have added some extra test coverage in rG862e8d7e5520. From what I could tell, the updated costs (when treating negpower2 same as power2) look OK.

RKSimon updated this revision to Diff 380887.Oct 20 2021, 3:25 AM

Thanks @dmgreen - updated with aarch64 test changes

RKSimon updated this revision to Diff 380920.Oct 20 2021, 5:56 AM
RKSimon edited the summary of this revision. (Show Details)

Add X86 mul by -pow2 handling

RKSimon updated this revision to Diff 380929.Oct 20 2021, 6:09 AM
RKSimon edited the summary of this revision. (Show Details)

Add SLP handling

RKSimon updated this revision to Diff 380953.Oct 20 2021, 7:43 AM

udiv/urem by -pow2 isn't as simple as shift/mask - fallback to the existing x86 udiv/urem by constant costs

AArch64 side looks OK to me, if you can find someone who knows the X86 part.

RKSimon planned changes to this revision.Oct 21 2021, 2:57 AM

I'm still looking at refining the x86 costs