public inbox for gcc@gcc.gnu.org
 help / color / mirror / Atom feed
* Question about gimple code during optimizing if-conversion
@ 2023-10-14  5:15 Hanke Zhang
  2023-10-14 15:49 ` Andrew Pinski
  0 siblings, 1 reply; 3+ messages in thread
From: Hanke Zhang @ 2023-10-14  5:15 UTC (permalink / raw)
  To: gcc

Hi, I'm working on optimizing if-conversion for my own business
recently. I got a problem here.

I tried to optimize it in such a case, for example, when a conditional
statement block has only if statement and no else statement, the
source C code looks like this:

int* foo; // assume this has been initialized
int c = rand(), t = rand(), size = 10000000;
for (int i = 0; i < size; i++) {
  if (foo[i] & (1 << c)) foo[i] ^= (1 << t);
}

Part of its corresponding gimple is optimized like this before if-conversion:

  <bb 5>:
  # i_71 = PHI <i_39(12), 0(9)>
  # ivtmp_9 = PHI <ivtmp_73(12), 10000000(9)>
  _5 = (long unsigned int) i_71;
  _6 = _5 * 4;
  _7 = foo_23 + _6;
  _8 = *_7;
  shifttmp_75 = _8 & shifttmp_76;
  if (shifttmp_75 != 0)
    goto <bb 6>; [50.00%]
  else
    goto <bb 14>; [50.00%]

  <bb 14> [local count: 531502205]:
  goto <bb 7>; [100.00%]

  <bb 6> [local count: 531502204]:
  _12 = _8 ^ _11;
  *_7 = _12;

  <bb 7> [local count: 1063004409]:
  i_39 = i_71 + 1;
  ivtmp_73 = ivtmp_9 - 1;
  if (ivtmp_73 != 0)
    goto <bb 12>; [99.00%]
  else
    goto <bb 8>; [1.00%]

I want to add some statements to gimple to make it like adding an else
block to the source code.

// What I expected:
int* foo; // assume this has been initialized
int c = rand(), t = rand(), size = 10000000;
for (int i = 0; i < size; i++) {
  if (foo[i] & (1 << c)) foo[i] ^= (1 << t);
+  else foo[i] = foo[i];  // I want to add a statment here !!!!!
}

And of course I can't change the source code for real, so I can only
add a pass in front of if-conversion to modify the gimple.

For the example above, I know that I have to add them in the block
'<bb14>', but what confuses me is that I don't know what kind of
statement to add to be legal due to my poor experience.

I try to add something like this below, but the compile error just
happened. So I'm here for help. What kind of statements should I add
here?

<bb 14> [local count: 531502205]:
+ *_7 = *_7
 goto <bb 7>; [100.00%]

Finally, The reason I did this was to avoid MASK_STORE generation,
because it might add an if branch in the final assembly which I don't
like it to be. And after such a modification, if-conversion should
have been changed it to the form of a ternary expression, which would
reduce the occurrence of branches after final vectorization and
produce more efficient code.

Or there if is a better way to get rid of MASK_STORE, please tell me
about that. :)

Thanks
Hanke Zhang

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: Question about gimple code during optimizing if-conversion
  2023-10-14  5:15 Question about gimple code during optimizing if-conversion Hanke Zhang
@ 2023-10-14 15:49 ` Andrew Pinski
  2023-10-14 16:55   ` Jeff Law
  0 siblings, 1 reply; 3+ messages in thread
From: Andrew Pinski @ 2023-10-14 15:49 UTC (permalink / raw)
  To: Hanke Zhang; +Cc: gcc

On Fri, Oct 13, 2023 at 10:16 PM Hanke Zhang via Gcc <gcc@gcc.gnu.org> wrote:
>
> Hi, I'm working on optimizing if-conversion for my own business
> recently. I got a problem here.
>
> I tried to optimize it in such a case, for example, when a conditional
> statement block has only if statement and no else statement, the
> source C code looks like this:
>
> int* foo; // assume this has been initialized
> int c = rand(), t = rand(), size = 10000000;
> for (int i = 0; i < size; i++) {
>   if (foo[i] & (1 << c)) foo[i] ^= (1 << t);
> }
>
> Part of its corresponding gimple is optimized like this before if-conversion:
>
>   <bb 5>:
>   # i_71 = PHI <i_39(12), 0(9)>
>   # ivtmp_9 = PHI <ivtmp_73(12), 10000000(9)>
>   _5 = (long unsigned int) i_71;
>   _6 = _5 * 4;
>   _7 = foo_23 + _6;
>   _8 = *_7;
>   shifttmp_75 = _8 & shifttmp_76;
>   if (shifttmp_75 != 0)
>     goto <bb 6>; [50.00%]
>   else
>     goto <bb 14>; [50.00%]
>
>   <bb 14> [local count: 531502205]:
>   goto <bb 7>; [100.00%]
>
>   <bb 6> [local count: 531502204]:
>   _12 = _8 ^ _11;
>   *_7 = _12;
>
>   <bb 7> [local count: 1063004409]:
>   i_39 = i_71 + 1;
>   ivtmp_73 = ivtmp_9 - 1;
>   if (ivtmp_73 != 0)
>     goto <bb 12>; [99.00%]
>   else
>     goto <bb 8>; [1.00%]
>
> I want to add some statements to gimple to make it like adding an else
> block to the source code.
>
> // What I expected:
> int* foo; // assume this has been initialized
> int c = rand(), t = rand(), size = 10000000;
> for (int i = 0; i < size; i++) {
>   if (foo[i] & (1 << c)) foo[i] ^= (1 << t);
> +  else foo[i] = foo[i];  // I want to add a statment here !!!!!
> }
>
> And of course I can't change the source code for real, so I can only
> add a pass in front of if-conversion to modify the gimple.
>
> For the example above, I know that I have to add them in the block
> '<bb14>', but what confuses me is that I don't know what kind of
> statement to add to be legal due to my poor experience.
>
> I try to add something like this below, but the compile error just
> happened. So I'm here for help. What kind of statements should I add
> here?
>
> <bb 14> [local count: 531502205]:
> + *_7 = *_7
>  goto <bb 7>; [100.00%]
>
> Finally, The reason I did this was to avoid MASK_STORE generation,
> because it might add an if branch in the final assembly which I don't
> like it to be. And after such a modification, if-conversion should
> have been changed it to the form of a ternary expression, which would
> reduce the occurrence of branches after final vectorization and
> produce more efficient code.
>
> Or there if is a better way to get rid of MASK_STORE, please tell me
> about that. :)

So there are 2 issues with this transformation which you need to take
into account.
1) C11/C++11 threading model (-fallow-store-data-races is needed)
2) foo could be read only and cause a trap if written to. if the
branch is never taken there would be no writes



Thanks,
Andrew Pinski

>
> Thanks
> Hanke Zhang

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: Question about gimple code during optimizing if-conversion
  2023-10-14 15:49 ` Andrew Pinski
@ 2023-10-14 16:55   ` Jeff Law
  0 siblings, 0 replies; 3+ messages in thread
From: Jeff Law @ 2023-10-14 16:55 UTC (permalink / raw)
  To: Andrew Pinski, Hanke Zhang; +Cc: gcc



On 10/14/23 09:49, Andrew Pinski via Gcc wrote:
> On Fri, Oct 13, 2023 at 10:16 PM Hanke Zhang via Gcc <gcc@gcc.gnu.org> wrote:
>>
>> Hi, I'm working on optimizing if-conversion for my own business
>> recently. I got a problem here.
>>
>> I tried to optimize it in such a case, for example, when a conditional
>> statement block has only if statement and no else statement, the
>> source C code looks like this:
>>
>> int* foo; // assume this has been initialized
>> int c = rand(), t = rand(), size = 10000000;
>> for (int i = 0; i < size; i++) {
>>    if (foo[i] & (1 << c)) foo[i] ^= (1 << t);
>> }
>>
>> Part of its corresponding gimple is optimized like this before if-conversion:
>>
>>    <bb 5>:
>>    # i_71 = PHI <i_39(12), 0(9)>
>>    # ivtmp_9 = PHI <ivtmp_73(12), 10000000(9)>
>>    _5 = (long unsigned int) i_71;
>>    _6 = _5 * 4;
>>    _7 = foo_23 + _6;
>>    _8 = *_7;
>>    shifttmp_75 = _8 & shifttmp_76;
>>    if (shifttmp_75 != 0)
>>      goto <bb 6>; [50.00%]
>>    else
>>      goto <bb 14>; [50.00%]
>>
>>    <bb 14> [local count: 531502205]:
>>    goto <bb 7>; [100.00%]
>>
>>    <bb 6> [local count: 531502204]:
>>    _12 = _8 ^ _11;
>>    *_7 = _12;
>>
>>    <bb 7> [local count: 1063004409]:
>>    i_39 = i_71 + 1;
>>    ivtmp_73 = ivtmp_9 - 1;
>>    if (ivtmp_73 != 0)
>>      goto <bb 12>; [99.00%]
>>    else
>>      goto <bb 8>; [1.00%]
>>
>> I want to add some statements to gimple to make it like adding an else
>> block to the source code.
>>
>> // What I expected:
>> int* foo; // assume this has been initialized
>> int c = rand(), t = rand(), size = 10000000;
>> for (int i = 0; i < size; i++) {
>>    if (foo[i] & (1 << c)) foo[i] ^= (1 << t);
>> +  else foo[i] = foo[i];  // I want to add a statment here !!!!!
>> }
>>
>> And of course I can't change the source code for real, so I can only
>> add a pass in front of if-conversion to modify the gimple.
>>
>> For the example above, I know that I have to add them in the block
>> '<bb14>', but what confuses me is that I don't know what kind of
>> statement to add to be legal due to my poor experience.
>>
>> I try to add something like this below, but the compile error just
>> happened. So I'm here for help. What kind of statements should I add
>> here?
>>
>> <bb 14> [local count: 531502205]:
>> + *_7 = *_7
>>   goto <bb 7>; [100.00%]
>>
>> Finally, The reason I did this was to avoid MASK_STORE generation,
>> because it might add an if branch in the final assembly which I don't
>> like it to be. And after such a modification, if-conversion should
>> have been changed it to the form of a ternary expression, which would
>> reduce the occurrence of branches after final vectorization and
>> produce more efficient code.
>>
>> Or there if is a better way to get rid of MASK_STORE, please tell me
>> about that. :)
> 
> So there are 2 issues with this transformation which you need to take
> into account.
> 1) C11/C++11 threading model (-fallow-store-data-races is needed)
> 2) foo could be read only and cause a trap if written to. if the
> branch is never taken there would be no writes
Right.   See ifcvt_memrefs_wont_trap.  That probably could be extended 
to capture additional cases.  But I'm not sure that'll be sufficient for 
Hanke's problem.

Jeff


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2023-10-14 16:55 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-10-14  5:15 Question about gimple code during optimizing if-conversion Hanke Zhang
2023-10-14 15:49 ` Andrew Pinski
2023-10-14 16:55   ` Jeff Law

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).