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

[Florian Hahn via Phabricator via llvm-commits]

fhahn added inline comments.


================
Comment at: llvm/lib/Target/X86/X86TargetTransformInfo.cpp:3260
 
-        Cost +=
-            getMemoryOpCost(Opcode, SubTy, Alignment, AddressSpace, CostKind);
-
-        std::pair<InstructionCost, MVT> LST =
-            TLI->getTypeLegalizationCost(DL, SubTy);
-        if (!LST.second.isVector()) {
-          APInt DemandedElts =
-              APInt::getBitsSet(NumElem, NumElemDone, NumElemDone + Factor);
-          Cost += getScalarizationOverhead(VTy, DemandedElts,
-                                           Opcode == Instruction::Load,
-                                           Opcode == Instruction::Store);
-        }
+  auto *VTy = dyn_cast<FixedVectorType>(Src);
+
----------------
Should scalable vectors ever reach here? I would expect something went very wrong in that case, so maybe we can use `cast` instead?


================
Comment at: llvm/lib/Target/X86/X86TargetTransformInfo.cpp:3273
+  const int EltTyBits = DL.getTypeSizeInBits(EltTy);
+  assert(((EltTyBits > 0) && (EltTyBits % 8 == 0)) &&
+         "Expected byte-size types");
----------------
This assert triggers if a vector of `i1` (e.g. `<16 x i1>`) is passed. `EltTyBits` will be `1`. 

The assert can be triggered by running `opt -cost-model -analyze -mtriple=x86_64-unknown-linux-gnu` on
```
define void @foo(<16 x i1> %v, <16 x i1>* %ptr) {
  store <16 x i1> %v, <16 x i1>* %ptr
  ret void
}
```

See https://llvm.godbolt.org/z/jxPvdGEW4 for a run-able version.






Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100684