[PATCH] D37277: [TTI] Fix getGEPCost() for geps with a single operand

Florian Hahn via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Aug 31 12:09:12 PDT 2017


fhahn accepted this revision.
fhahn added a comment.
This revision is now accepted and ready to land.

LGTM. Thanks for addressing this issue, this is definitely an improvement and much better than passing an uninitialized value to isLegalAddressingMode.

I think to mirror the behavior of isLegalAddressingMode in this file as closely as possible, we should return TTI::TCC_Free, if !BaseGV, else TTI::TCC_Basic.

The other minor nit is just about the position of the early exit, and I think either position is fine.



================
Comment at: include/llvm/Analysis/TargetTransformInfoImpl.h:706
+    // If this GEP has a single operand, the basis, it should be cheap.
+    if (Operands.empty())
+      return TTI::TCC_Free;
----------------
If I understand correctly, we use this early exit to cover the case where TargetType is NULL, because we didn't execute any iterations of the loop above.

I think it would be slightly clearer to just move the early exit before TargetType, with a comment that this covers the case where TargetType would be null.


https://reviews.llvm.org/D37277





More information about the llvm-commits mailing list