Thanks for your comments.
On Wed, Dec 14, 2011 at 10:06 PM, Sun Chan <sun.c...@gmail.com> wrote:

> I don't understand the rationale for this fix. You are saying that the
> dead branch is not dce'd, and lno does not like it. So, why not fix dce? A
> dead branch is dead branch, why this workaround?
> Sun
>
>
This segmentation is in the preopt, but intermediate optimized result as:
IF {line: 1/131}
U4INTCONST 1 (0x1)
THEN
...
ELSE
BLOCK
END_BLOCK
END_IF
is acceptable, not 100% optimized but still in its own right and later
mainopt can still eliminate the dead branch.

for a long term, we should fix proactive_loop_transform, since we should
not always assume the intermediate input in perfect form.

Coding style in open64.net pointing this out:

no assumption across

   - No assumption across different phases


As a temporary workaround, we assume the previously code work quite well
except for cases like bug912, so, we make more effort on limit the changes,
i.e, more constraint on the change. if we add the MAINOPT condition, the
branch get eliminted by later DCE in preopt, then proactive_loop_transform
happy. we get things done.

Regards
Gang


> On Wed, Dec 14, 2011 at 12:34 PM, Gang Yu <yugang...@gmail.com> wrote:
>
>> Hi,
>>
>>    Would a gatekeeper please help review the fix for bug934 , regression
>> caused by r3843?
>>
>> segment-fault function snippet of  bug case:
>>
>> void LibRaw:: tiff_set (ushort *ntag,
>>  ushort tag, ushort type, int count, int val)
>> {
>>   struct tiff_tag *tt;
>>   int c;
>>   if (type < 3 && count <= 4)
>>     for (c=0; c < 4; c++) tt->val.c[c] = val >> (c << 3);
>> }
>> void LibRaw:: tiff_head (struct tiff_hdr *th, int full)
>> {
>>   if (full) {
>>     tiff_set (&th->ngps, 3, 2, 2, (imgdata.other.gpsdata)[30]);
>>     tiff_set (&th->ngps, 4, 5, 3, ((char *)(&(th->gps[6])) - (char *)th));
>>     tiff_set (&th->ngps, 5, 1, 1, (imgdata.other.gpsdata)[31]);
>>   }
>>   int c, row, col, soff, rstep, cstep;
>>   for (row=0; row < (imgdata.sizes.height); row++, soff += rstep) {
>>   }
>> }
>>
>> at O3 ,  tiff_set is inlined into tiff_set, at PREOPT_LNO_PHASE,
>> expression
>> if (type <3 && count <=4), i.e, whirl expression
>>
>>    U4U2LDID 0 <st 89> T<7,.predef_U2,2>
>>    U4INTCONST 2 (0x2)
>>   U4U4LE
>>    I4I4LDID 0 <st 90> T<4,.predef_I4,4>
>>    I4INTCONST 4 (0x4)
>>   U4I4LE
>>  I4CAND
>> U4I4CVT
>> after codemap, is transfered to
>> >  LDC U4 1 <u=2 cr19> flags:0x0 b=-1
>> > U4I4CVT <u=2 cr33> isop_flags:0xc0000 flags:0x1 b=-1
>> >OPR_FALSEBR 6914 b=-1 flags:0x2 pj2 Sid-1
>> DCE does not delete the unreachable branch under this condition, which
>> then emits back to H whirl
>> as
>>   IF {line: 1/131}
>>    U4INTCONST 1 (0x1)
>>   THEN
>>      ...
>>   ELSE
>>      BLOCK
>>      END_BLOCK
>>   END_IF
>>
>> this will make proactive transformation phase in PREOPT_LNO1_PHASE
>> uncomfort, then it asserts.
>>
>> Suggested patch, keep the orginal handling of U4I4CVT const , only change
>> under the MAINOPT_phase.
>>
>>
>> osprey/be/opt/opt_htable.cxx    -- 148c4e2..6499b99 100644
>> --- a/osprey/be/opt/opt_htable.cxx
>> +++ b/osprey/be/opt/opt_htable.cxx
>> @@ -2273,7 +2273,9 @@ CODEMAP::Canon_cvt(WN       *wn,
>>         Get_mtype_class(OPCODE_desc(op))) != 0 &&
>>        MTYPE_size_min(OPCODE_rtype(op)) ==
>> MTYPE_size_min(OPCODE_desc(op)) &&
>>        // bug912 open64.net. Do not delete U4I4CVT if his kid is a
>> constant
>> -      (!(OPCODE_rtype(op) == MTYPE_U4 &&
>> +      // bug934 open64.net. Only do this at MAINOPT_PHASE.
>> +      (!(_phase == MAINOPT_PHASE &&
>> +         OPCODE_rtype(op) == MTYPE_U4 &&
>>           OPCODE_desc(op) == MTYPE_I4 &&
>>           ccr->Tree() == NULL)))
>>      return propagated;
>>
>> Would a gatekeeper help a review?
>>
>> BTW, thanks for Xiaojing for report the bug.
>>
>> Regards
>> Gang
>>
>>
>> ------------------------------------------------------------------------------
>> Cloud Computing - Latest Buzzword or a Glimpse of the Future?
>> This paper surveys cloud computing today: What are the benefits?
>> Why are businesses embracing it? What are its payoffs and pitfalls?
>> http://www.accelacomm.com/jaw/sdnl/114/51425149/
>> _______________________________________________
>> Open64-devel mailing list
>> Open64-devel@lists.sourceforge.net
>> https://lists.sourceforge.net/lists/listinfo/open64-devel
>>
>>
>
------------------------------------------------------------------------------
10 Tips for Better Server Consolidation
Server virtualization is being driven by many needs.  
But none more important than the need to reduce IT complexity 
while improving strategic productivity.  Learn More! 
http://www.accelacomm.com/jaw/sdnl/114/51507609/
_______________________________________________
Open64-devel mailing list
Open64-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/open64-devel

Reply via email to