Thanks for your comment. Actually, no need to add such a comment, as it is not special case. We haven't add all the other instructions into this list because all of them haven't use CMP/SEL/... in it. Unaligned store is just one of those instructions, and you can check the gen_insn_selection.cpp, there is even no deadicated unaligned store function.
> -----Original Message----- > From: Luo, Xionghu > Sent: Thursday, October 23, 2014 3:58 PM > To: Gong, Zhigang; [email protected] > Cc: Gong, Zhigang > Subject: RE: [Beignet] [PATCH] GBE: fix regression caused by simple block > optimization. > > LGTM. > Add comments for why unaligned store instruction not excluded before push, > please. > Thanks. > > Luo Xionghu > Best Regards > > -----Original Message----- > From: Beignet [mailto:[email protected]] On Behalf Of > Zhigang Gong > Sent: Thursday, October 23, 2014 2:26 PM > To: [email protected] > Cc: Gong, Zhigang > Subject: [Beignet] [PATCH] GBE: fix regression caused by simple block > optimization. > > Almost all 64bit related instructions and unaligned load instruction should be > complex instruction. We need to exclude them from simple block. > > Signed-off-by: Zhigang Gong <[email protected]> > --- > backend/src/backend/gen_insn_selection.cpp | 28 > ++++++++++++++++++++++++++++ > 1 file changed, 28 insertions(+) > > diff --git a/backend/src/backend/gen_insn_selection.cpp > b/backend/src/backend/gen_insn_selection.cpp > index 1a37ef1..605fdd5 100644 > --- a/backend/src/backend/gen_insn_selection.cpp > +++ b/backend/src/backend/gen_insn_selection.cpp > @@ -407,6 +407,8 @@ namespace gbe > void matchBasicBlock(const ir::BasicBlock &bb, uint32_t insnNum); > /*! a simple block can use predication instead of if/endif*/ > bool isSimpleBlock(const ir::BasicBlock &bb, uint32_t insnNum); > + /*! an instruction has a QWORD family src or dst operand. */ > + bool hasQWord(const ir::Instruction &insn); > /*! A root instruction needs to be generated */ > bool isRoot(const ir::Instruction &insn) const; > > @@ -1494,6 +1496,20 @@ namespace gbe > return false; > } > > + bool Selection::Opaque::hasQWord(const ir::Instruction &insn) { > + for (uint32_t i = 0; i < insn.getSrcNum(); i++) { > + const ir::Register reg = insn.getSrc(i); > + if (getRegisterFamily(reg) == ir::FAMILY_QWORD) > + return true; > + } > + for (uint32_t i = 0; i < insn.getDstNum(); i++) { > + const ir::Register reg = insn.getDst(i); > + if (getRegisterFamily(reg) == ir::FAMILY_QWORD) > + return true; > + } > + return false; > + } > + > bool Selection::Opaque::isSimpleBlock(const ir::BasicBlock &bb, uint32_t > insnNum) { > > // FIXME should include structured innermost if/else/endif @@ -1511,6 > +1527,18 @@ namespace gbe > insn.getOpcode() == ir::OP_SIMD_ALL || > insn.getOpcode() == ir::OP_ELSE) > return false; > + > + // Most of the QWord(long) related instruction introduce some CMP or > + // more than 10 actual instructions at latter stage. > + if (hasQWord(insn)) > + return false; > + > + // Unaligned load may introduce CMP instruction. > + if ( insn.isMemberOf<ir::LoadInstruction>()) { > + const ir::LoadInstruction &ld = ir::cast<ir::LoadInstruction>(insn); > + if (!ld.isAligned()) > + return false; > + } > } > > // there would generate a extra CMP instruction for predicated BRA with > extern flag, > -- > 1.8.3.2 > > _______________________________________________ > Beignet mailing list > [email protected] > http://lists.freedesktop.org/mailman/listinfo/beignet _______________________________________________ Beignet mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/beignet
