Andrew Cagney wrote:
>>> Jason Molenda wrote:
>>> Hi Jeff,
>>>
>>> On Dec 17, 2003, at 1:15 PM, Jeff Johnston wrote:
>>>
>>> There are a few bugs in do_mixed_source_and_assembly() when dealing 
>>> with the ia64.
>>>
>>>
>>> I hit one of the two bugs you're fixing last week, namely the double 
>>> call to close off the list/tuple.  My first fix looked like yours, 
>>> but I think a slight reworking of the loop is very desirable here.  
>>> Right now this loop looks approximately like this:
>>>
>>>
>>>    Set close_list to 1 (the list/tuple will be closed at the end of 
>>> this loop)
>>>
>>>    First entry of a new source line number:
>>>      Start a "src_and_asm_line" tuple, print the source line.
>>>      Start a "line_asm_insn" list where instructions will be emitted.
>>>      If we're not at the end of the mle array, and the next mle 
>>> entry's source line number is not greater than the current source 
>>> line entry, DON'T close off the list (close_list = 0)
>>>
>>>    Print the assembly instructions for the current address range.
>>>
>>>    If close_list is 1, close the tuple/list.
>>>
>>>
>>> Which is a very convoluted way of writing a loop, and more 
>>> importantly, this only works correctly for one or two assembly ranges 
>>> for a single source line.  If you have a third, on the 2nd iter the 
>>> tuple/list are closed and then on the 3rd you close them again.
>>>
>>> Instead, this is more clear:
>>>
>>>
>>>   First entry of a new source line number:
>>>      Start a "src_and_asm_line" tuple, print the source line.
>>>      Start a "line_asm_insn" list where instructions will be emitted.
>>>
>>>   Print the assembly instructions for the current address range.
>>>
>>>   If we're at the end of the mle array or the next entry in the array 
>>> is a new source line, close off the list and tuple.
>>>
>>>
>>> I also took the opportunity to combine two conditional statements - I 
>>> won't push hard for that part of the change, but the rest is a clear 
>>> improvement IMHO.
>>>
>>> We only had this code path executed when we were using a compiler 
>>> with a bug in it internally, so it's not too easy for me to 
>>> reproduce/test this.  I've tried to combine my patch and yours 
>>> against the current FSF TOT.  What do you think of my suggested 
>>> additional change?  Can you try it on the IA64 test case you have?
>>>
>>>
>>> Jason
>>>
> 
>>
>> Works fine.  I like the revision you made.  I don't know if the 
>> powers-that-be will review this patch before the New Year. 
> 
> 
> Since it works for you Jeff, yes, why not (just a shame that the powers 
> that didn't take a holiday didn't think to review it).
> 
> Andrew
> 
>

Thanks Andrew.  I have checked in the accompanying modified patch.  The 
modification was simply to set the initial values of the cleanups to be
null_cleanup rather than NULL per the discussion we had on the other thread.  As 
well, I updated the copyright year.

-- Jeff J.

>