[PATCH] D100684: [X86][CostModel] X86TTIImpl::getMemoryOpCost(): rewrite vector handling again

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added inline comments.


================
Comment at: llvm/lib/Target/X86/X86TargetTransformInfo.cpp:3249
+  const int EltTyBytes = EltTyBits / 8;
+  assert(EltTyBytes != 0);
+
----------------
RKSimon wrote:
> Can this assert happen?
No, the previous assert would have fired already.
This is more to document the expected post-condition.
It is inferrable from the previous assert, but i thought this is more direct.


================
Comment at: llvm/lib/Target/X86/X86TargetTransformInfo.cpp:3258
+  int NumEltRemaining = SrcNumElt;
+  auto NumEltDone = [&]() { return SrcNumElt - NumEltRemaining; };
+
----------------
RKSimon wrote:
> This feels like it should be unnecessary.....
I don't want to have `int NumEltDone`, because then we will have to update two things (it and NumEltRemaining).
Should i inline it? I though giving it a name would make code more readable.


================
Comment at: llvm/lib/Target/X86/X86TargetTransformInfo.cpp:3262
+  const int MaxLegalOpSizeBytes = LT.second.getSizeInBits() / 8;
+  assert(MaxLegalOpSizeBytes != 0);
+
----------------
RKSimon wrote:
> assert message?
Same as previous assert, can't really happen, but i thought making it explicit might be good..


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100684