[PATCH] D78922: [CostModel] Remove getExtCost
Dave Green via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed May 20 09:16:13 PDT 2020
dmgreen added inline comments.
================
Comment at: llvm/include/llvm/CodeGen/BasicTTIImpl.h:717
+
+ if (getTLI()->isExtFree(I))
+ return 0;
----------------
samparker wrote:
> dmgreen wrote:
> > I don't believe this will be correct from the vectorizer. It can pass a context instruction that has different types to the final IR due to it truncating at the same time it vectorizes. It is generally unsound to rely on I being accurate.
> I'm sure there's going to be many cases like that, and the vectorizer can always choose to not pass the instruction.
Hmm. You may be correct there in the end. I was kind of hoping that we could keep the context instruction but only look at opcodes. I don't think even that will remain truly correct for very long though.
I believe that adding this would at least cause inaccuracies in the costmodelling over what we have right now, possibly regressions in some cases. Not passing I through from the vectorizer (and any other place where we couldn't trust the context) would seem to need a different change to me though.
================
Comment at: llvm/test/Analysis/CostModel/AArch64/cast.ll:30
; CHECK-NEXT: Cost Model: Found an estimated cost of 0 for instruction: %r23 = trunc i32 undef to i16
-; CHECK-NEXT: Cost Model: Found an estimated cost of 1 for instruction: %r24 = sext i32 undef to i64
+; CHECK-NEXT: Cost Model: Found an estimated cost of 0 for instruction: %r24 = sext i32 undef to i64
; CHECK-NEXT: Cost Model: Found an estimated cost of 0 for instruction: %r25 = zext i32 undef to i64
----------------
Also, and I may be forgetting something because I thought these looked fine the last time I looked, but why would a sext i32->i64 be free? Would it not need a sxtw?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D78922/new/
https://reviews.llvm.org/D78922
More information about the llvm-commits
mailing list