Re: [Open64-devel] Code Review request for bug966 [CG]
Brought to you by:
ributzka,
suneeljain
Menu
▾
▴
Re: [Open64-devel] Code Review request for bug966 [CG]
|
From: Jian-Xin L. <la...@gm...> - 2012-06-28 10:56:28
|
They were converted into U4 in HSSA.
2012/6/28 Sun Chan <sun...@gm...>:
> but it is i8 in this case
> Sun
>
> On Thu, Jun 28, 2012 at 6:36 PM, Jian-Xin Lai <la...@gm...> wrote:
>> I remember I tried this when I was doing the 4-way merge. This code
>> came from Pathscale and it's quite different from IA-64/NVISA/SL. At
>> that time, we tried to unify them but failed. There was a lot of
>> failures if we set the type of the const other than U4/I8.
>>
>> 2012/6/28 Sun Chan <sun...@gm...>:
>>> 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
>>>>> >
>>>>
>>>>
>>>
>>> ------------------------------------------------------------------------------
>>> 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
>>
>>
>>
>> --
>> Regards,
>> Lai Jian-Xin
--
Regards,
Lai Jian-Xin
|