[PATCH] D84679: [VPlan] Disconnect VPValue and VPUser.

[Dave Green via Phabricator via llvm-commits]

dmgreen added a comment.

> With this patch, `VPRecipeBase` is also a `VPUser` and a `VPValue`, it just inherits from both directly, rather than indirectly through `VPValue`. Note that 'replacing all uses with & co' are provided by VPValue, so all instances that could previously be RAUW'd can still be RAUW'd.  The only major difference should be that in the future we may need some extra code to get the uses of a `VPUser`.

Do you mean VPInstruction is a VPUser and a VPValue? I really meant that _all_ recipes were VPValues and VPUsers, moving it to VPRecipeBase.

Like I said, the code I have is very rough, and I'm not sure it's in any way mutually exclusive with going in this direction. I will try and rebase what I have onto your patches, working in the same way. This is the kind of transform it was doing (which was working pretty well it seemed, minus the costing not being accounted for):

  static bool tryCombineToMulh(VPBasicBlock::reverse_iterator &It, VPlanPtr &Plan,
                               VPRecipeBuilder &RecipeBuilder) {
    VPWidenRecipe *RV = dyn_cast<VPWidenRecipe>(&*It);
    if (!RV)
      return false;
  
    // trunc(lsr(mul(ext, ext)), BW)) -> mulhs/mulhu
    Type *Ty = RV->getType();
    unsigned BW = Ty->getScalarSizeInBits();
  
    VPValue *Mul;
    // TODOD: One use checks?
    if (!match(RV, vp_Trunc(vp_LShr(vp_Value(Mul), vp_SpecificInt(BW)))))
      return false;
  
    VPValue *A, *B;
    unsigned Opcode = 0;
    if (match(Mul, vp_Mul(vp_SExt(vp_Value(A)), vp_SExt(vp_Value(B)))))
      Opcode = VPInstruction::Mulhs;
    else if (match(Mul, vp_Mul(vp_ZExt(vp_Value(A)), vp_ZExt(vp_Value(B)))))
      Opcode = VPInstruction::Mulhu;
    else
      return false;
  
    unsigned MulBW = Mul->getType()->getScalarSizeInBits();
    if (MulBW > BW * 2)
      return false;
  
    if (A->getType() != Ty || B->getType() != Ty) // TODOD: Generalize
      return false;
  
    auto *Mulh = new VPInstruction(Opcode, {A, B});
    RV->getParent()->insert(Mulh, RV->getIterator());
    RV->replaceAllUsesWith(Mulh);
    RV->recursivelyDeleteUnusedRecipes();
    It = Mulh->getReverseIterator();
    return true;
  }
  
  bool LoopVectorizationPlanner::adjustVPlanForVectorPatterns(
      VPlanPtr &Plan, VPRecipeBuilder &RecipeBuilder) {
    bool Changed = false;
    for (VPBlockBase *Block : depth_first(Plan->getEntry())) {
      if (auto *BB = dyn_cast<VPBasicBlock>(Block)) {
        for(VPBasicBlock::reverse_iterator It = BB->rbegin(), E = BB->rend(); It != E; ++It) {
          if (true /*TODOD: TTI->hasVectorPattern(Mulh)*/)
            Changed |= tryCombineToMulh(It, Plan, RecipeBuilder);
        }
      }
    }
  
    return Changed;
  }

As all the vp_Mul etc are really looking at VPWidenRecipe, perhaps this can work simply enough. The idea was to probably make them match VPWidenRecipe or VPInstruction. recursivelyDeleteUnusedRecipes might be tougher, but perhaps still possible. I will see.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84679