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 <dvor...@suse.cz>
>>>
>>> * 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 :-)
jeff
