[PATCH] D146737: [AMDGPU] Default component broadcast store

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUInstCombineIntrinsic.cpp:381
+    if (auto *IEI = dyn_cast<InsertElementInst>(CurInst)) {
+      auto *IEIIndex = dyn_cast<ConstantInt>(IEI->getOperand(2));
+      auto *Comp = IEI->getOperand(1);
----------------
Unchecked dyn_cast, need a test with a variable vector index


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUInstCombineIntrinsic.cpp:360
+  std::vector<int> Components(VWidth, -1);
+  std::map<Value *, int> MapValueInt;
+  int LastElem = 0;
----------------
matejam wrote:
> matejam wrote:
> > arsenm wrote:
> > > Don't use std::map. Also, can this just be a SmallVector in the reverse direction per component?
> > I will look into that.
> What about MapVector?
> I'm not sure how would I use SmallVector to implement a map?
I meant it's the value to vector indexes? You could just go the other direction and find the value from the index


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

https://reviews.llvm.org/D146737