Re: [Open64-devel] Code Review request for bug966 [CG]
Brought to you by:
ributzka,
suneeljain
From: Sun C. <sun...@gm...> - 2012-06-27 22:30:10
|
Just do an experiment, turn off this thing for this particular case to prevent the cvt inserted, see what would happen? Sun On Wed, Jun 27, 2012 at 10:32 PM, Xiaojing Zhang <xia...@gm...> wrote: > Hi, Sun > > > for case1, the IR dump after if_conv phase in WOPT below: > > I4I4LDID 0 <st 1> T<4,.predef_I4,4> > > I4INTCONST 0 (0x0) > > I4I4NE > > I8INTCONST 4294967292 (0xfffffffc) > > I8INTCONST 4294967288 (0xfffffff8) > > I8SELECT > > I8STID 49 <st 3> T<5,.predef_I8,4> {line: 1/29} {freq: 0, ln: 29, col: 0} > > > but after the function 'Set_dtyp_const_val' in op_htable.h, the res type of > const will be turned to U4. so I8U4CVT is needed to make sure both Kid 1 and > Kid 2 have the same res type as SELECT. > > 696 void Set_dtyp_const_val(MTYPE dt, UINT64 v) { Is_True(Kind() == > CK_CONST, > > 697 > ("CODEREP::Set_dtyp_const_val, illegal kind")); > > 698 if (v == (v << 32) >> 32) > > 699 _dtyp = MTYPE_U4; > > 700 else _dtyp = MTYPE_I8; > > 701 u2.isconst.const_val = v; } > > > > 2012/6/27 Sun Chan <sun...@gm...> >> >> why should there be a I8U4CVT? the LDC could have been an I8 to start out >> with. >> Sun >> >> On Wed, Jun 27, 2012 at 4:56 PM, Xiaojing Zhang <xia...@gm...> >> wrote: >> > >> > Hi, >> > >> > Could a gatekeeper please help review the fix for bug966? >> > (http://bugs.open64.net/show_bug.cgi?id=966) >> > >> > >> > 1. Test case >> > >> > The following two cases failed with command “-m32 -O2”. >> > >> > ### Assertion failure at line 255 of >> > /fc/home/zxj/open64/x86_DBG/osprey/../../osprey/be/cg/x8664/expand.cxx: >> > >> > ### Compiler Error in file zsh.c during Code_Expansion phase: >> > >> > ### Expand_Cmov: invalid src2 >> > >> > >> > >> > Case1: >> > >> > __extension__ typedef long long int __quad_t; >> > >> > __extension__ typedef __quad_t __off64_t; >> > >> > __extension__ typedef int __pid_t; >> > >> > typedef struct >> > >> > { >> > >> > struct >> > >> > { >> > >> > } __data; >> > >> > }; >> > >> > typedef struct _IO_FILE FILE; >> > >> > typedef struct >> > >> > { >> > >> > } _G_fpos64_t; >> > >> > enum __codecvt_result >> > >> > { >> > >> > __codecvt_ok, >> > >> > }; >> > >> > extern int fseeko (FILE *__stream, __off64_t __off, int __whence) >> > __asm__ >> > ("" "fseeko64"); >> > >> > struct utmpx >> > >> > { >> > >> > __pid_t ut_pid; >> > >> > }; >> > >> > getlogtime(struct utmpx *u, int inout) >> > >> > { >> > >> > FILE *in; >> > >> > struct utmpx uu; >> > >> > int first = 1; >> > >> > do { >> > >> > if (fseeko(in, ((first) ? -1 : -2) * sizeof(struct utmpx), 1)) { >> > >> > } >> > >> > first = 0; >> > >> > } >> > >> > while (memcmp(&uu, u, sizeof(uu))); >> > >> > } >> > >> > Case2: >> > >> > typedef unsigned char uint8_t; >> > >> > typedef unsigned int uint32_t; >> > >> > typedef unsigned long long int uint64_t; >> > >> > extern memddd(unsigned int); >> > >> > void build_avp(uint32_t avp_vendor, uint64_t in_value_len) >> > >> > { >> > >> > if (avp_vendor != 0) >> > >> > { >> > >> > in_value_len = in_value_len +4; >> > >> > } >> > >> > { >> > >> > memddd(in_value_len); >> > >> > } >> > >> > } >> > >> > >> > >> > 2. Analyses >> > >> > The assertion failure is triggered during Code_Expansion phase, when >> > expanding I8SELECT, the pair TN of its kid is null, so it hit the >> > assertion. >> > But the whirl IR after WOPT phase was not correct, so the root cause >> > should >> > be in WOPT phase. In SELECT, Both Kid 1 and Kid 2 must have res as the >> > result type. In case1, the Dtyp of kid1 and kid2 turn to U4 from I8 in >> > SSA::Create_CODEMAP phase; in case2, the Dtyp of kid 1 turn to U4 from >> > U8 in >> > BITWISE_DCE phase. So the fix method here is to generate CVT for the >> > kids to >> > make sure their Dtyp are the same with SELECT. >> > >> >>>In Expand_Cmov:255 >> > >> > Build_OP(top, result, src, rflags, ops); >> > >> > Set_OP_cond_def_kind(OPS_last(ops), OP_ALWAYS_COND_DEF); >> > >> > if (result2 != NULL) { >> > >> > è Is_True(src2 != NULL, ("Expand_Cmov: invalid src2")); >> > >> > Build_OP(top, result2, src2, rflags, ops); >> > >> > Set_OP_cond_def_kind(OPS_last(ops), OP_ALWAYS_COND_DEF); >> > >> > } >> > >> > the src2 value is set at: >> > >> > TN* result_hi = NULL; >> > >> > TN* true_tn_hi = NULL; >> > >> > if (OP_NEED_PAIR(select_type)) { >> > >> > result_hi = Get_TN_Pair(result); >> > >> > è true_tn_hi = Get_TN_Pair(true_tn); >> > >> > } >> > >> > For case 1, In function Exp_Select_And_Condition, the value of kid2 >> > (false_tn, TN152) of SELECT is copied to result, and it also create a >> > pair >> > TN for the result. >> > >> > [ 0, 0] TN159 :- mov32 TN152 ; copy >> > >> > [ 0, 0] TN160 :- mov32 TN153 ; copy >> > >> > [ 0, 0] TN33(%rflags) :- test32 TN157 TN157 ; >> > >> > then, it will call Expand_Comv to decide whether to copy the kid1 >> > (true_tn,TN149) of SELECT to result, and that depend on the value of the >> > compare. >> > >> > after generate CVT for the kids, the ops below: >> > >> > [ 0, 0] TN159 :- mov32 TN152 ; copy, >> > >> > [ 0, 0] TN160 :- mov32 TN153 ; copy >> > >> > [ 0, 0] TN33(%rflags) :- test32 TN157 TN157 ; >> > >> > [ 0, 0] TN159 :- cmovne TN149 TN33(%rflags) ; cond_def >> > >> > [ 0, 0] TN160 :- cmovne TN150 TN33(%rflags) ; cond_def >> > >> > >> > >> > 3. IR Dump >> > >> > Case1: >> > >> > Original IR: >> > >> > I4I4LDID 54 <1,4,.preg_I4> T<4,.predef_I4,4> # first >> > >> > U4INTCONST 0 (0x0) >> > >> > I4I4NE >> > >> > U4U4LDID 55 <1,8,.preg_U4> T<8,.predef_U4,4> # lpre_cst >> > >> > U4U4LDID 56 <1,8,.preg_U4> T<8,.predef_U4,4> # lpre_cst >> > >> > I8SELECT >> > >> > I8STID 4 <2,12,.SP> T<5,.predef_I8,4> {line: 1/29} >> > >> > After adding CVT: >> > >> > LDID I4 I4 sym1v4 0 ty=402 <u=1 cr13> flags:0x8 b=-1 #first >> > >> > LDC U4 0 <u=4 cr17> flags:0x0 b=-1 >> > >> > I4I4NE <u=1 cr18> isop_flags:0x0 flags:0x0 b=-1 >> > >> > LDC U4 4294967292 <u=1 cr19> flags:0x0 b=-1 >> > >> > I8U4CVT <u=1 cr21> isop_flags:0x0 flags:0x0 b=-1 >> > >> > LDC U4 4294967288 <u=1 cr20> flags:0x0 b=-1 >> > >> > I8U4CVT <u=1 cr22> isop_flags:0x0 flags:0x0 b=-1 >> > >> > I8SELECT <u=1 cr23> isop_flags:0x0 flags:0x0 b=-1 >> > >> > >> > Case2: >> > >> > Original IR: >> > >> > LDID U4 U4 sym1v2 0 ty=802 <u=1 cr1> flags:0x10 b=-1 #avp_vendor >> > >> > LDC U4 0 <u=1 cr11> flags:0x0 b=-1 >> > >> > I4U4NE <u=1 cr12> isop_flags:0xcc040 flags:0x1 b=E1 >> > >> > LDID U8 U8 sym6v3 49 ty=902 <u=0 cr25> flags:0x0 b=-1 >> > >> > U4U8CVT <u=0 cr31> isop_flags:0x0 flags:0x0 b=-1 >> > >> > LDC U4 4 <u=0 cr13> flags:0x0 b=-1 >> > >> > U4ADD <u=0 cr32> isop_flags:0x0 flags:0x1 b=E2 >> > >> > LDID U8 U8 sym6v3 49 ty=902 <u=0 cr25> flags:0x0 b=-1 >> > >> > U8SELECT <u=1 cr33> isop_flags:0x0 flags:0x1 b=E41 >> > >> > After adding CVT: >> > >> > LDID U4 U4 sym1v2 0 ty=802 <u=1 cr1> flags:0x10 b=-1 #avp_vendor >> > >> > LDC U4 0 <u=1 cr11> flags:0x0 b=-1 >> > >> > I4U4NE <u=1 cr12> isop_flags:0xcc040 flags:0x1 b=E1 >> > >> > LDID U8 U8 sym6v3 49 ty=902 <u=2 cr25> flags:0x0 b=-1 >> > >> > U4U8CVT <u=1 cr31> isop_flags:0x0 flags:0x0 b=-1 >> > >> > LDC U4 4 <u=1 cr13> flags:0x0 b=-1 >> > >> > U4ADD <u=1 cr32> isop_flags:0x0 flags:0x1 b=E2 >> > >> > U8U4CVT <u=1 cr33> isop_flags:0x0 flags:0x0 b=-1 >> > >> > LDID U8 U8 sym6v3 49 ty=902 <u=2 cr25> flags:0x0 b=-1 >> > >> > U8SELECT <u=1 cr34> isop_flags:0x0 flags:0x1 b=E41 >> > >> > >> > >> > 4. Patch: >> > >> > Index: osprey/be/opt/opt_htable.cxx >> > >> > =================================================================== >> > >> > --- osprey/be/opt/opt_htable.cxx (revision 3952) >> > >> > +++ osprey/be/opt/opt_htable.cxx (working copy) >> > >> > @@ -3737,6 +3737,17 @@ >> > >> > cr->Set_call_op_aux_id (WN_st_idx(wn)); >> > >> > break; >> > >> > #endif >> > >> > + // Fix bug966: for SELECT, both Kid 1 and Kid 2 must have res as >> > the >> > result type >> > >> > + case OPR_SELECT: >> > >> > + for (INT index = 1; index < cr->Kid_count(); index++) { >> > >> > + CODEREP *opnd = cr->Opnd(index); >> > >> > + if (cr->Dtyp() != opnd->Dtyp()) { >> > >> > + OPCODE opc = OPCODE_make_op(OPR_CVT, cr->Dtyp(), >> > opnd->Dtyp()); >> > >> > + CODEREP *cvt_cr = Add_unary_node(opc, opnd); >> > >> > + cr->Set_opnd(index, cvt_cr); >> > >> > + } >> > >> > + } >> > >> > + break; >> > >> > } >> > >> > >> > >> > BOOL do_canonicalization = TRUE; >> > >> > Index: osprey/be/opt/opt_bdce.cxx >> > >> > =================================================================== >> > >> > --- osprey/be/opt/opt_bdce.cxx (revision 3952) >> > >> > +++ osprey/be/opt/opt_bdce.cxx (working copy) >> > >> > @@ -1381,8 +1381,20 @@ >> > >> > } >> > >> > else new_cr->Set_opnd(i, cr->Opnd(i)); >> > >> > } >> > >> > + // Fix bug966: for SELECT, both Kid 1 and Kid 2 must have res as >> > the >> > result type >> > >> > + opr = cr->Opr(); >> > >> > + if (opr == OPR_SELECT) { >> > >> > + for (INT index = 1; index < new_cr->Kid_count(); index++) { >> > >> > + CODEREP *opnd = new_cr->Opnd(index); >> > >> > + if (new_cr->Dtyp() != opnd->Dtyp()) { >> > >> > + OPCODE opc = OPCODE_make_op(OPR_CVT, new_cr->Dtyp(), >> > opnd->Dtyp()); >> > >> > + CODEREP *cvt_cr = Htable()->Add_unary_node(opc, opnd); >> > >> > + new_cr->Set_opnd(index, cvt_cr); >> > >> > + need_rehash = TRUE; >> > >> > + } >> > >> > + } >> > >> > + } >> > >> > // check if current node can be deleted >> > >> > - opr = cr->Opr(); >> > >> > if (opr == OPR_CVTL) { >> > >> > if (((Livebits(cr) & ~Bitmask_of_size(cr->Offset())) == 0) || >> > >> > Redundant_cvtl(MTYPE_is_signed(cr->Dtyp()), >> > >> > >> > >> > ------------------------------------------------------------------------------ >> > Live Security Virtual Conference >> > Exclusive live event will cover all the ways today's security and >> > threat landscape has changed and how IT managers can respond. >> > Discussions >> > will include endpoint security, mobile security and the latest in >> > malware >> > threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/ >> > _______________________________________________ >> > Open64-devel mailing list >> > Ope...@li... >> > https://lists.sourceforge.net/lists/listinfo/open64-devel >> > > > |