On 6/6/19 11:54 PM, Jeff Law wrote:
> On 6/6/19 1:42 PM, Andrew MacLeod wrote:
>> On 6/6/19 1:20 PM, Jeff Law wrote:
>>> On 6/6/19 7:02 AM, Andrew MacLeod wrote:
>>>> On 6/6/19 6:20 AM, Martin Liška wrote:
>>>>> Hi.
>>>>>
>>>>> The code is dead:
>>>>>
>>>>> 757 char *
>>>>> 758 get_lsm_tmp_name (tree ref, unsigned n, const char *suffix)
>>>>> 759 {
>>>>> 760 char ns[2];
>>>>> 761
>>>>> 762 lsm_tmp_name_length = 0;
>>>>> 763 gen_lsm_tmp_name (ref);
>>>>> 764 lsm_tmp_name_add ("_lsm");
>>>>> 765 if (n < 10)
>>>>> 766 {
>>>>> 767 ns[0] = '0' + n;
>>>>> 768 ns[1] = 0;
>>>>> 769 lsm_tmp_name_add (ns);
>>>>> 770 }
>>>>> 771 return lsm_tmp_name;
>>>>> 772 if (suffix != NULL)
>>>>> 773 lsm_tmp_name_add (suffix);
>>>>> 774 }
>>>>>
>>>>> Andrew is it a typo or an issue?
>>>>> Thanks,
>>>>> Martin
>>>> Dunno. It was written in 2005.
>>>> 2005-08-16 Zdenek Dvorak <[email protected]>
>>>>
>>>> * tree-ssa-loop-im.c (MAX_LSM_NAME_LENGTH, lsm_tmp_name,
>>>> lsm_tmp_name_length): New.
>>>> (lsm_tmp_name_add, gen_lsm_tmp_name, get_lsm_tmp_name): New
>>>> functions.
>>>> (schedule_sm): Use get_lsm_tmp_name instead of "lsm_tmp".
>>>>
>>>> The whole thing is a little odd since you cant get more than 10 tmp
>>>> names without suddenly all being the same name.
>>>>
>>>> I dont know anything about the code, my guess is the return should be
>>>> after the 'if'. the only callers appears to pass ~0 as the value for N.
>>>> execute_sm_if_changed_flag_set() adds '_flag' as a suffix and
>>>> execute_sm() calls it without the suffix.
>>>>
>>>> My guess is the return should be moved to the bottom so that those 2 get
>>>> different names, so it could be a problem as it is. Someone who know
>>>> the loop code better could comment..
>>> So it looks like the code was "sensible" here:
>>>
>>>> ad4a85adaf8f (rakdver 2007-05-24 16:09:26 +0000 1844)
>>>> get_lsm_tmp_name (tree ref, unsigned n)
>>>> 840580de9cd8 (rakdver 2005-08-17 14:00:52 +0000 1845) {
>>>> ad4a85adaf8f (rakdver 2007-05-24 16:09:26 +0000 1846) char ns[2];
>>>> ad4a85adaf8f (rakdver 2007-05-24 16:09:26 +0000 1847)
>>>> 840580de9cd8 (rakdver 2005-08-17 14:00:52 +0000 1848)
>>>> lsm_tmp_name_length = 0;
>>>> 840580de9cd8 (rakdver 2005-08-17 14:00:52 +0000 1849)
>>>> gen_lsm_tmp_name (ref);
>>>> 840580de9cd8 (rakdver 2005-08-17 14:00:52 +0000 1850)
>>>> lsm_tmp_name_add ("_lsm");
>>>> ad4a85adaf8f (rakdver 2007-05-24 16:09:26 +0000 1851) if (n < 10)
>>>> ad4a85adaf8f (rakdver 2007-05-24 16:09:26 +0000 1852) {
>>>> ad4a85adaf8f (rakdver 2007-05-24 16:09:26 +0000 1853) ns[0]
>>>> = '0' + n;
>>>> ad4a85adaf8f (rakdver 2007-05-24 16:09:26 +0000 1854) ns[1]
>>>> = 0;
>>>> ad4a85adaf8f (rakdver 2007-05-24 16:09:26 +0000 1855)
>>>> lsm_tmp_name_add (ns);
>>>> ad4a85adaf8f (rakdver 2007-05-24 16:09:26 +0000 1856) }
>>>> 840580de9cd8 (rakdver 2005-08-17 14:00:52 +0000 1857) return
>>>> lsm_tmp_name;
>>>> 840580de9cd8 (rakdver 2005-08-17 14:00:52 +0000 1858) }
>>> But got scrambled as part of your change to move things around here:
>>>
>>>> commit f86b328b32d171e9f2c5274ea7bc2dd3e92ad827
>>>> Author: amacleod <amacleod@138bc75d-0d04-0410-961f-82ee72b054a4>
>>>> Date: Wed Oct 9 13:09:23 2013 +0000
>>>>
>>>> * tree-flow.h: Move some protoypes. Include new
>>>> tree-ssa-loop.h.
>>>> (struct affine_iv, struct tree_niter_desc): Move to
>>>> tree-ssa-loop.h.
>>>> (enum move_pos): Move to tree-ssa-loop-im.h
>>>> * cfgloop.h: Move some prototypes.
>>>> (gcov_type_to_double_int): relocate from
>>>> tree-ssa-loop.niter.c.
>>>> * tree-flow-inline.h (loop_containing_stmt): Move to
>>>> tree-ssa-loop.h.
>>>> * tree-ssa-loop.h: New File. Include other
>>>> tree-ssa-loop-*.h files.
>>>> (struct affine_iv, struct tree_niter_desc): Relocate
>>>> from tree-flow.h.
>>>> (loop_containing_stmt): Relocate from tree-flow-inline.h.
>>>> * tree-ssa-loop-ch.c: (do_while_loop_p): Make static.
>>>> * tree-ssa-loop-im.c (for_each_index): Move to
>>>> tree-ssa-loop.c.
>>>> (enum move_pos): Relocate here.
>>>> (lsm_tmp_name_add, gen_lsm_tmp_name, get_lsm_tmp_name):
>>>> Move to
>>>> tree-ssa-loop.c.
>>> [ ... ]
>>>
>>>
>>> Jeff
>>
>> and more importantly,
>>
>> (get_lsm_tmp_name): Relocate and add suffix parameter.
>>
>> must have been some sort of factoring going on.. and those lines got
>> missed. doesnt seem to have ever afffected anything eh :-)
>>
>> Anyway, then yes, the return should be moved to the bottom to the
>> function where it belongs :-),
> Patch to do that pre-approved with the usual testing :-)
Good, I've done that as r272029.
Thanks,
Martin
>
> jeff
>
