[PATCH] D76124: [TTI] Remove getOperationCost

Sanjay Patel via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Apr 8 09:45:14 PDT 2020


spatel added inline comments.


================
Comment at: llvm/include/llvm/Analysis/TargetTransformInfoImpl.h:861
+      if (getCastInstrCost(Opcode, Ty, OpTy, I) == TTI::TCC_Free ||
+          TargetTTI->getCastInstrCost(Opcode, Ty, OpTy, I) == TTI::TCC_Free)
+        return TTI::TCC_Free;
----------------
samparker wrote:
> spatel wrote:
> > This still isn't clear to me (let me know if I'm misunderstanding and/or reading too much into this). 
> > 
> > getCastInstrCost() returns some integer value as a cost. Do we really want to use the "TCC_Free" enum value as an alias for "0" in the comparison statement? If we are going to use that definition, then do we need to update the documentation comments to specify that TCC_Free is a designated return value and fixed at zero?
> My immediate response was, 'isn't that the purpose of an enum?' But I now I think I know what you mean, though I don't feel that it really matters here and isn't out-of-line with how this layer already operates. Maybe it could be addressed at the same time as fixing the unsigned/int mess..? Also, the next step here will be to return the raw value anyway, without the condition, to get a more accurate answer.
Yes, I assumed that we were trying to move away from the bogus enum values, so it seemed backwards to create another use of those. If that's just a temporary splotch on the way to the better fix, it's ok with me.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D76124/new/

https://reviews.llvm.org/D76124





More information about the llvm-commits mailing list