[PATCH] D12035: [ARM] Improve cost model to handle sdiv by a pow-of-two.

Geoff Berry via llvm-commits llvm-commits at lists.llvm.org
Mon Aug 17 10:53:34 PDT 2015


gberry added a comment.

It seems like we may want to factor this code out (perhaps into a protected TargetTransformInfo function) so that all targets that use this DAG combine (which looks to be most of them) can use it as well.

I see a couple of potential issues with the cost estimation itself:

1. the DAG combine adds an additional SUB if the divisor is negative which isn't accounted for
2. the OperandValueKinds (OpInfo1, OpInfo2) are used for all of the operations, but these do not describe the actual operands for the intermediate instructions since they are not operating on the original operands of the SDIV.  For example, the ADD instruction is an add of two intermediate results which we know nothing about (so they should be marked as OK_AnyValue), but operand 2 of the ADD will be marked as OK_UniformConstant.  This may not make a difference for this particular pattern on the targets in question, but could potentially be incorrect for other targets.


Repository:
  rL LLVM

http://reviews.llvm.org/D12035





More information about the llvm-commits mailing list