This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH 0/3][POPCOUNT]


Hi Jeff,

Thanks for the comments.

On 23 June 2018 at 02:06, Jeff Law <law@redhat.com> wrote:
> On 06/22/2018 03:11 AM, Kugan Vivekanandarajah wrote:
>> When we set niter with maybe_zero, currently final_value_relacement
>> will not happen due to expression_expensive_p not handling. Patch 1
>> adds this.
>>
>> With that we have the following optimized gimple.
>>
>>   <bb 2> [local count: 118111601]:
>>   if (b_4(D) != 0)
>>     goto <bb 3>; [89.00%]
>>   else
>>     goto <bb 4>; [11.00%]
>>
>>   <bb 3> [local count: 105119324]:
>>   _2 = (unsigned long) b_4(D);
>>   _9 = __builtin_popcountl (_2);
>>   c_3 = b_4(D) != 0 ? _9 : 1;
>>
>>   <bb 4> [local count: 118111601]:
>>   # c_12 = PHI <c_3(3), 0(2)>
>>
>> I assume that 1 in  b_4(D) != 0 ? _9 : 1; is OK (?) because when the
>> latch execute zero times for b_4 == 0 means that the body will execute
>> ones.
> ISTM that DOM ought to have simplified the conditional, unless there's
> some other way to get to bb3.  We know that b_4 is nonzero and thus c_3
> must have the value _9.
As of now, dom is not optimizing it. With the attached hack, it can be made to.

>
>
>>
>> The issue here is, since we are checking if (b_4(D) != 0) before
>> entering the loop means we don't need to set maybe_zero. Patch 2
>> handles this.
>>
>> With that we have
>>   <bb 2> [local count: 118111601]:
>>   if (b_4(D) != 0)
>>     goto <bb 3>; [89.00%]
>>   else
>>     goto <bb 4>; [11.00%]
>>
>>   <bb 3> [local count: 105119324]:
>>   _2 = (unsigned long) b_4(D);
>>   _9 = __builtin_popcountl (_2);
>>
>>   <bb 4> [local count: 118111601]:
>>   # c_12 = PHI <0(2), _9(3)>
>>
>> As advised earlier, patch 3 adds phiopt support to remove this.
> So if DOM or some other pass fixed up the assignment to c_3 I'd hope we
> wouldn't set maybe_zero.
>
> So I'd like to start by first determining if we should have already
> simplified the COND_EXPR in bb3.  Do you have a testcase for that?
Sorry, It is hidden in patch 3 (attaching now). You will need patch 1 as well.

Thanks,
Kugan

>
>
> jeff

Attachment: dom.txt
Description: Text document

int PopCount (long b) {
    int c = 0;

    while (b) {
	b &= b - 1;
	c++;
    }
    return c;
}

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]