"Kewen.Lin" <li...@linux.ibm.com> writes:
> Hi Andrea,
>
> on 2020/9/4 锟斤拷锟斤拷8:11, Andrea Corallo wrote:
>> Hi all,
>>
>> just a small patch removing a piece of unreachable code in
>> 'vect_estimate_min_profitable_iters' given the condition
>> (LOOP_VINFO_USING_PARTIAL_VECTORS_P (loop_vinfo)) is always true as
>> checked just above.
>>
>
> FWIW, I had the same confusion when I saw the code at the first time,
> the comments at outer if "??? The "if" arm ..." and the commit messages
> seem to give some hints. IIUC, the code in this outer "if" was
> implemented to handle all cases but that time was stage 4, so it's
> conservative to be guarded as fully-predicated only to avoid the
> potential risks. I was imagining that it would finally end up
> by removing this outer if condition guard and the outer else.
Right. So the idea was to show what the code would look like if we
removed the outer “if” condition. We would then always calculate the
estimate based on the minimum number of profitable vector iterations
first, then convert that into a minimum number of profitable scalar
iterations.
That still seems like a promising future direction. It would make the
estimate calculations more consistent and make it easier to compare the
cost of full-vector loops with the cost of partial-vector loops.
> But not sure why it didn't change in the following stage1.
Excess lameness on my part, sorry.
> Maybe Richard S. (the author) will have some comments.
TBH I'd prefer for the patch to be reverted. Maybe we could add
a version of the existing:
/* ??? The "if" arm is written to handle all cases; see below for what
we would do for !LOOP_VINFO_USING_PARTIAL_VECTORS_P. */
comment above:
else if (LOOP_VINFO_USING_PARTIAL_VECTORS_P (loop_vinfo))
{
E.g.:
/* ??? This "else if" arm is written to handle all cases; see below for
what we would do for !LOOP_VINFO_USING_PARTIAL_VECTORS_P. */
As:
/* This is a repeat of the code above, but with + SOC rather
than - SOC. */
says, this block is essentially repeating the logic from the earlier block,
so at the time it didn't seem like a good idea to duplicate the commentary.
Either way, the two blocks should be kept in sync, so if we want to remove
the path from this block, we should remove it from the earlier block too.
But my preference would be to keep the path in both blocks.
I'm not foolish enough to promise that I'll test/benchmark removing the
outer “if” condition in this stage 1 cycle, but I'll at least try to find
time…
Thanks,
Richard
