[Beignet] [PATCH] Fix the bug of multi deleting of load instruction in lowering

[Zhigang Gong]

LGTM, pushed, thanks.

On Mon, Jan 20, 2014 at 11:28:43AM +0800, junyan.he at inbox.com wrote:
> From: Junyan He <junyan.he at linux.intel.com>
> 
> When the load instruction has multi-value destinations, the load
> instruction in buildConstantPush function will be replaced many
> times and which can cause the potential problems.
> ---
>  backend/src/ir/instruction.cpp |    9 +++++++++
>  backend/src/ir/instruction.hpp |    2 ++
>  backend/src/ir/lowering.cpp    |   12 +++++++++---
>  3 files changed, 20 insertions(+), 3 deletions(-)
> 
> diff --git a/backend/src/ir/instruction.cpp b/backend/src/ir/instruction.cpp
> index 5e3cd84..a3e096f 100644
> --- a/backend/src/ir/instruction.cpp
> +++ b/backend/src/ir/instruction.cpp
> @@ -1442,6 +1442,15 @@ END_FUNCTION(Instruction, Register)
>      fn.deleteInstruction(this);
>    }
>  
> +  void Instruction::insert(Instruction *prev, Instruction ** new_ins) {
> +    Function &fn = prev->getFunction();
> +    Instruction *insn = fn.newInstruction(*this);
> +    insn->parent = prev->parent;
> +    append(insn, prev);
> +    if (new_ins)
> +      *new_ins = insn;
> +  }
> +
>    bool Instruction::hasSideEffect(void) const {
>      return opcode == OP_STORE ||
>             opcode == OP_TYPED_WRITE ||
> diff --git a/backend/src/ir/instruction.hpp b/backend/src/ir/instruction.hpp
> index c827b9a..696d1d9 100644
> --- a/backend/src/ir/instruction.hpp
> +++ b/backend/src/ir/instruction.hpp
> @@ -170,6 +170,8 @@ namespace ir {
>      void replace(Instruction *other) const;
>      /*! Remove the instruction from the instruction stream */
>      void remove(void);
> +    /* Insert the instruction after the previous one. */
> +    void insert(Instruction *prev, Instruction ** new_ins = NULL);
>      /*! Indicates if the instruction belongs to instruction type T. Typically, T
>       *  can be BinaryInstruction, UnaryInstruction, LoadInstruction and so on
>       */
> diff --git a/backend/src/ir/lowering.cpp b/backend/src/ir/lowering.cpp
> index 49b6e06..638b15b 100644
> --- a/backend/src/ir/lowering.cpp
> +++ b/backend/src/ir/lowering.cpp
> @@ -225,6 +225,8 @@ namespace ir {
>      for (const auto &loadAddImm : seq) {
>        LoadInstruction *load = cast<LoadInstruction>(loadAddImm.load);
>        const uint32_t valueNum = load->getValueNum();
> +      bool replaced = false;
> +      Instruction *ins_after = load; // the instruction to insert after.
>        for (uint32_t valueID = 0; valueID < valueNum; ++valueID) {
>          const Type type = load->getValueType();
>          const RegisterFamily family = getFamily(type);
> @@ -249,12 +251,16 @@ namespace ir {
>          // register is never written. We must however support the register
>          // replacement in the instruction interface to be able to patch all the
>          // instruction that uses "reg"
> -        const Instruction mov = ir::MOV(type, reg, pushed);
> -        mov.replace(load);
> -        dead.insert(load);
> +        Instruction mov = ir::MOV(type, reg, pushed);
> +        mov.insert(ins_after, &ins_after);
> +        replaced = true;
>        }
> +
> +      if (replaced)
> +        dead.insert(load);
>      }
>  
> +    REMOVE_INSN(load)
>      REMOVE_INSN(add)
>      REMOVE_INSN(loadImm)
>    }
> -- 
> 1.7.9.5
> 
> _______________________________________________
> Beignet mailing list
> Beignet at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/beignet