[Beignet] [PATCH] GBE: correct the instruction replacement logic in scalarize pass.

[Yang, Rong R]

LGTM, thanks.

> -----Original Message-----
> From: Beignet [mailto:beignet-bounces at lists.freedesktop.org] On Behalf Of
> Zhigang Gong
> Sent: Thursday, April 2, 2015 07:22
> To: beignet at lists.freedesktop.org
> Cc: Gong, Zhigang
> Subject: [Beignet] [PATCH] GBE: correct the instruction replacement logic in
> scalarize pass.
> 
> When we want to delete an old instruction and replace it with the new one,
> we only call the LLVM IR's replace function which is not sufficient for the
> scalarize pass, as we also keep some local reference int eh vecVals map. We
> need to replace all of those local reference also.
> 
> Otherwise, the deleted values may be used in the subsequent instructions
> which causes fatal error latter.
> 
> Signed-off-by: Zhigang Gong <zhigang.gong at intel.com>
> ---
>  backend/src/llvm/llvm_scalarize.cpp | 43
> +++++++++++++++++++++++++++++--------
>  1 file changed, 34 insertions(+), 9 deletions(-)
> 
> diff --git a/backend/src/llvm/llvm_scalarize.cpp
> b/backend/src/llvm/llvm_scalarize.cpp
> index b657246..694b008 100644
> --- a/backend/src/llvm/llvm_scalarize.cpp
> +++ b/backend/src/llvm/llvm_scalarize.cpp
> @@ -234,6 +234,20 @@ namespace gbe {
>      }
> 
>      DenseMap<Value*, VectorValues> vectorVals;
> +    struct VecValElement{
> +      VecValElement(VectorValues *v, uint32_t i) : vecVals(v), id(i) {}
> +      VectorValues *vecVals;
> +      uint32_t id;
> +    };
> +    DenseMap<Value*, SmallVector<VecValElement, 16>> usedVecVals;
> +
> +    void setComponent(VectorValues &vecVals, uint32_t c, llvm::Value* val) {
> +      vecVals.setComponent(c, val);
> +      usedVecVals[val].push_back(VecValElement(&vecVals, c));
> +    }
> +
> +    void replaceAllUsesOfWith(Instruction* from, Instruction* to);
> +
>      Module* module;
>      IRBuilder<>* builder;
> 
> @@ -448,7 +462,7 @@ namespace gbe {
>        callArgs.push_back(ConstantInt::get(intTy, i));
> 
>        res = builder->CreateCall(f, callArgs);
> -      vVals.setComponent(i, res);
> +      setComponent(vVals, i, res);
>      }
>    }
> 
> @@ -468,7 +482,7 @@ namespace gbe {
> 
>        Instruction* res = createScalarInstruction(inst, callArgs);
> 
> -      vVals.setComponent(i, res);
> +      setComponent(vVals, i, res);
>        builder->Insert(res);
>      }
>    }
> @@ -549,7 +563,7 @@ namespace gbe {
>        int select = sv->getMaskValue(i);
> 
>        if (select < 0) {
> -        vVals.setComponent(i, UndefValue::get(GetBasicType(sv-
> >getOperand(0))));
> +        setComponent(vVals, i,
> + UndefValue::get(GetBasicType(sv->getOperand(0))));
>          continue;
>        }
> 
> @@ -564,7 +578,7 @@ namespace gbe {
>          selectee = sv->getOperand(1);
>        }
> 
> -      vVals.setComponent(i, getComponent(select, selectee));
> +      setComponent(vVals, i, getComponent(select, selectee));
>      }
> 
>      return true;
> @@ -611,7 +625,7 @@ namespace gbe {
>      for (int i = 0; i < GetComponentCount(insn); ++i) {
>        Value *cv = ConstantInt::get(intTy, i);
>        Value *EI = builder->CreateExtractElement(insn, cv);
> -      vVals.setComponent(i, EI);
> +      setComponent(vVals, i, EI);
>      }
>    }
> 
> @@ -680,6 +694,7 @@ namespace gbe {
>        setAppendPoint(bt);
>        extractFromVector(bt);
>      }
> +
>      return false;
>    }
> 
> @@ -695,6 +710,16 @@ namespace gbe {
>      return false;
>    }
> 
> +  void Scalarize::replaceAllUsesOfWith(Instruction* from, Instruction* to) {
> +    GBE_ASSERT(from != NULL);
> +    if (from == to)
> +      return;
> +    for (auto &it : usedVecVals[from])
> +      setComponent(*(it.vecVals), it.id, to);
> +    usedVecVals[from].clear();
> +    from->replaceAllUsesWith(to);
> +  }
> +
>    bool Scalarize::scalarizeExtract(ExtractElementInst* extr)
>    {
>      //     %res = extractelement <n X ty> %foo, %i
> @@ -711,7 +736,7 @@ namespace gbe {
>      Value* v = getComponent(component, extr->getOperand(0));
>      if(extr == v)
>        return false;
> -    extr->replaceAllUsesWith(v);
> +    replaceAllUsesOfWith(dyn_cast<Instruction>(extr),
> + dyn_cast<Instruction>(v));
> 
>      return true;
>    }
> @@ -731,8 +756,8 @@ namespace gbe {
> 
>      VectorValues& vVals = vectorVals[ins];
>      for (int i = 0; i < GetComponentCount(ins); ++i) {
> -      vVals.setComponent(i, i == component ? ins->getOperand(1)
> -                                           : getComponent(i, ins->getOperand(0)));
> +      setComponent(vVals, i, i == component ? ins->getOperand(1)
> +                   : getComponent(i, ins->getOperand(0)));
>      }
> 
>      return true;
> @@ -816,9 +841,9 @@ namespace gbe {
>      }
> 
>      dce();
> -
>      incompletePhis.clear();
>      vectorVals.clear();
> +    usedVecVals.clear();
> 
>      delete builder;
>      builder = 0;
> --
> 1.9.1
> 
> _______________________________________________
> Beignet mailing list
> Beignet at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/beignet