[PATCH] D88060: [GISel]: Few InsertVecElt combines

Jay Foad via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Sep 30 05:00:05 PDT 2020


foad added inline comments.


================
Comment at: llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp:2413
+  assert(DstTy.isVector() && "Invalid G_INSERT_VECTOR_ELT?");
+  unsigned NumElts = DstTy.getNumElements();
+  auto Index = getConstantVRegVal(MI.getOperand(3).getReg(), MRI);
----------------
Everything after this point looks way overcomplicated and could be replaced with something like:
```
MatchInfo.resize(NumElts);
while (mi_match(
    CurrInst->getOperand(0).getReg(), MRI,
    m_GInsertVecElt(m_MInstr(TmpInst), m_Reg(TmpReg), m_ICst(IntImm)))) {
  if (IntImm >= NumElts)
    return false;
  if (!MatchInfo[IntImm])
    MatchInfo[IntImm] = TmpReg;
  CurrInst = TmpInst;
}
return TmpInst->getOpcode() == TargetOpcode::G_IMPLICIT_DEF;
```


================
Comment at: llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp:2491
+      MatchInfo.push_back(Op.getReg());
+    MatchInfo[*IdxCst] = EltReg;
+    return true;
----------------
Don't you have to defend against `*IdxCst` being out of range here too?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88060



More information about the llvm-commits mailing list