This is an archive of the discontinued LLVM Phabricator instance.

[ARM] Teach the Arm cost model that a Shift can be folded into other instructions
ClosedPublic

Authored by dmgreen on Dec 3 2019, 9:12 AM.

Details

Summary

This attempts to teach the cost model in Arm that code such as:

%s = shl i32 %a, 3
%a = and i32 %s, %b

Can under Arm or Thumb2 become:

and r0, r1, r2, lsl #3

So the cost of the shift can essentially be free. To do this without trying to artificially adjust the cost of the "and" instruction, it needs to get the users of the shl and check if they are a type of instruction that the shift can be folded into. And so it needs to have access to the actual instruction in getArithmeticInstrCost, which if available is added as an extra parameter much like getCastInstrCost.

We otherwise limit it to shifts with a single user, which should hopefully handle most of the cases. The list of instruction that the shift can be folded into include ADC, ADD, AND, BIC, CMP, EOR, MVN, ORR, ORN, RSB, SBC and SUB. This translates to Add, Sub, And, Or, Xor and ICmp.

Diff Detail

Event Timeline

dmgreen created this revision.Dec 3 2019, 9:12 AM
Herald added a project: Restricted Project. · View Herald TranscriptDec 3 2019, 9:12 AM

The Arm logic looks fine by me, but the tests look somewhat lacking! I'm not asking for testing all combinations, but do we have an example that we could use..?

llvm/include/llvm/Analysis/TargetTransformInfo.h
903–905

Should probably document the new optional argument.

spatel added inline comments.Dec 4 2019, 12:25 PM
llvm/include/llvm/Analysis/TargetTransformInfo.h
912

+1 to documenting the argument. Also, in InstSimplify and possibly some other places, this param is called the context instruction and spelled "CxtI". I've never liked that abbreviation, but that would still help the casual reader understand the intent.

llvm/lib/Target/ARM/ARMTargetTransformInfo.cpp
725–726

This can use "isShift(unsigned Opcode)" from Instruction.h - or the class member version: I->isShift().

dmgreen updated this revision to Diff 232736.Dec 8 2019, 6:28 AM

Added some extra testing and renamed I to CxtI.

spatel accepted this revision.Dec 8 2019, 7:04 AM

LGTM

llvm/include/llvm/Analysis/TargetTransformInfo.h
904

typo: CtxI -> CxtI
That's why I don't like that particular abbreviation - it doesn't "sound" right. :)

This revision is now accepted and ready to land.Dec 8 2019, 7:04 AM
nhaehnle removed a subscriber: nhaehnle.Dec 8 2019, 9:51 AM
This revision was automatically updated to reflect the committed changes.