2017-01-06 22:40 GMT+08:00 <xionghu....@intel.com>:
> From: Luo Xionghu <xionghu....@intel.com>
>
> the if opt could be a independent pass like function by checking the
> instruction state changes and special instructions like I64, mixed bit
> etc. this could reduce the code complexit of structure code.
>
> v2: as the GenInstructionState flag/subFlag default value is 0.0, so
> isSimpleBlock function return false if the insn state uses 0.1 as flag.
> This rule could make function more straight forward, no need to enum
> the special instructions except SEL_OP_SEL_CMP(no predication per spec).
>
> Signed-off-by: Luo Xionghu <xionghu....@intel.com>
> ---
> backend/src/CMakeLists.txt | 1 +
> backend/src/backend/gen_context.cpp | 8 +-
> backend/src/backend/gen_insn_selection.hpp | 2 +
> backend/src/backend/gen_insn_selection_if_opt.cpp | 119
> ++++++++++++++++++++++
> 4 files changed, 129 insertions(+), 1 deletion(-)
> create mode 100644 backend/src/backend/gen_insn_selection_if_opt.cpp
>
> diff --git a/backend/src/CMakeLists.txt b/backend/src/CMakeLists.txt
> index 6ff25e7..c98ab3d 100644
> --- a/backend/src/CMakeLists.txt
> +++ b/backend/src/CMakeLists.txt
> @@ -104,6 +104,7 @@ set (GBE_SRC
> backend/gen_insn_selection.cpp
> backend/gen_insn_selection.hpp
> backend/gen_insn_selection_optimize.cpp
> + backend/gen_insn_selection_if_opt.cpp
> backend/gen_insn_scheduling.cpp
> backend/gen_insn_scheduling.hpp
> backend/gen_insn_selection_output.cpp
> diff --git a/backend/src/backend/gen_context.cpp
> b/backend/src/backend/gen_context.cpp
> index 10e2c9e..769cd86 100644
> --- a/backend/src/backend/gen_context.cpp
> +++ b/backend/src/backend/gen_context.cpp
> @@ -3617,14 +3617,20 @@ namespace gbe
> kernel->curbeSize = ALIGN(kernel->curbeSize, GEN_REG_SIZE);
> }
>
> + BVAR(OCL_OUTPUT_BEFORE_OPT, false);
> BVAR(OCL_OUTPUT_SEL_IR, false);
> BVAR(OCL_OPTIMIZE_SEL_IR, true);
> + BVAR(OCL_OPTIMIZE_IF_IR, true);
>
I think OCL_OPTIMIZE_IF_BLOCK is more easy to understand.
> bool GenContext::emitCode(void) {
> GenKernel *genKernel = static_cast<GenKernel*>(this->kernel);
> sel->select();
> + sel->addID();
> + if (OCL_OUTPUT_BEFORE_OPT)
>
OCL_OUTPUT_BEFORE_OPT is not easy to understand, output what?
I think OCL_OUTPUT_SEL_IR_AFTER_SELECT is a little better.
> + outputSelectionIR(*this, this->sel, genKernel->getName());
> if (OCL_OPTIMIZE_SEL_IR)
> sel->optimize();
> - sel->addID();
> + if (OCL_OPTIMIZE_IF_IR)
> + sel->if_opt();
>
Hi Xiuli,
this addID is only needed by outputSelectionIR(), right?
if it is only meaningful to outputSelectionIR, I would prefer putting it
under if (OCL_OUTPUT_SEL_IR).
> if (OCL_OUTPUT_SEL_IR)
outputSelectionIR(*this, this->sel, genKernel->getName());
> schedulePreRegAllocation(*this, *this->sel);
> diff --git a/backend/src/backend/gen_insn_selection.hpp
> b/backend/src/backend/gen_insn_selection.hpp
> index f4cc948..88ae44f 100644
> --- a/backend/src/backend/gen_insn_selection.hpp
> +++ b/backend/src/backend/gen_insn_selection.hpp
> @@ -318,6 +318,8 @@ namespace gbe
> void optimize(void);
> uint32_t opt_features;
>
> + void if_opt(void);
> +
> /* Add insn ID for sel IR */
> void addID(void);
> const GenContext &getCtx();
> diff --git a/backend/src/backend/gen_insn_selection_if_opt.cpp
> b/backend/src/backend/gen_insn_selection_if_opt.cpp
> new file mode 100644
> index 0000000..026f5b9
> --- /dev/null
> +++ b/backend/src/backend/gen_insn_selection_if_opt.cpp
> @@ -0,0 +1,119 @@
> +
> +#include "backend/gen_insn_selection.hpp"
> +#include "backend/gen_context.hpp"
> +#include "ir/function.hpp"
> +#include "ir/liveness.hpp"
> +#include "ir/profile.hpp"
> +#include "sys/cvar.hpp"
> +#include "sys/vector.hpp"
> +#include <algorithm>
> +#include <climits>
> +#include <map>
> +
> +namespace gbe
> +{
> + class IfOptimizer
> + {
> + public:
> + IfOptimizer(const GenContext& ctx, SelectionBlock& selblock) :
> ctx(ctx), selBlock(selblock) {}
> + void run();
> + bool isSimpleBlock();
> + void removeSimpleIfEndif();
> + ~IfOptimizer() {}
> + protected:
> + const GenContext &ctx; //in case that we need it
> + SelectionBlock &selBlock;
> + bool optimized;
> + };
> +
> + bool IfOptimizer::isSimpleBlock() {
> +
> + if(selBlock.insnList.size() > 20)
> + return false;
> +
> + bool if_exits = false;
> + bool endif_exits = false;
> + for (auto &insn : selBlock.insnList) {
> + if (insn.opcode == SEL_OP_IF) {
> + if_exits = true;
> + continue;
> + }
> + if(if_exits) {
> + GenInstructionState curr = insn.state;
> + if (curr.execWidth == 1 || curr.predicate != GEN_PREDICATE_NONE
> || curr.flagIndex != 0 || (curr.flag == 0 && curr.subFlag == 1)) {
> + return false;
> + }
> +
> + if (insn.opcode == SEL_OP_ELSE) {
> + return false;
> + }
> +
> + if (insn.opcode == SEL_OP_SEL_CMP) {
> + return false;
> + }
> + }
> +
> + if (insn.opcode == SEL_OP_ENDIF) {
> + endif_exits = true;
> + break;
> + }
> + }
> +
> + if (!if_exits || !endif_exits)
> + return false;
> +
> + return true;
> + }
> +
> + void IfOptimizer::removeSimpleIfEndif() {
> + if(isSimpleBlock()) {
> + GenInstructionState curr;
> + bool if_find = false;
>
I think you need to re-write your code like below to make it more clean:
you need to be careful here to make the logic is the same as before.
auto iter = selBlock.insnList.begin();
while (iter != selBlock.insnList.end()) {
//remove if and endif, change instruction flags.
SelectionInstruction& insn = *iter;
if(insn.opcode == SEL_OP_IF && !if_find) {
iter = selBlock.insnList.erase(&insn);
if_find = true;
} else if (ENDIF) {
iter = selBlock.insnList.erase(&insn);
optimized = true;
} else {
if (if_find) {
insn.state.predicate = GEN_PREDICATE_NORMAL;
insn.state.flag = 0;
insn.state.subFlag = 1;
}
++iter;
}
}
+ for (auto iter = selBlock.insnList.begin(); iter !=
> selBlock.insnList.end(); iter++ ) {
> + //remove if and endif, change instruction flags.
> + SelectionInstruction& insn = *iter;
> + if(insn.opcode == SEL_OP_IF && !if_find) {
> + iter = selBlock.insnList.erase(&insn);
> + if (iter == selBlock.insnList.end()) {
> + break;
> + }
> + SelectionInstruction &next = *iter;
> + next.state.predicate = GEN_PREDICATE_NORMAL;
> + next.state.flag = 0;
> + next.state.subFlag = 1;
> + if_find = true;
> + continue;
> + }
> +
> + if(!if_find)
> + continue;
> +
> + insn.state.predicate = GEN_PREDICATE_NORMAL;
> + insn.state.flag = 0;
> + insn.state.subFlag = 1;
> + if (insn.opcode == SEL_OP_ENDIF) {
> + selBlock.insnList.erase(&insn);
> + optimized = true;
> + break;
> + }
> + }
> + }
> + }
> +
> + void IfOptimizer::run()
> + {
> + optimized = false;
> + removeSimpleIfEndif();
> + }
> +
> + void Selection::if_opt()
> + {
> + //do basic block level optimization
> + for (SelectionBlock &block : *blockList) {
> + IfOptimizer ifopt(getCtx(), block);
> + ifopt.run();
> + }
> +
> + }
> +} /* namespace gbe */
> +
> --
> 2.5.0
>
> _______________________________________________
> Beignet mailing list
> Beignet@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/beignet
>
_______________________________________________
Beignet mailing list
Beignet@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/beignet
