https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109393

--- Comment #19 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to cuilili from comment #16)
> I found a regression with commit: "match: Change (A * B) + (-C) to (B - C/A)
> * A, if C multiple of A [PR109393]".
> 
> Godbolt link: https://godbolt.org/z/qq46h7Wjf
> 
> -----------------------------------
> Test case:
> 
> int func1(int *a, int j) {
>   int k = j - 1;
>   return a[j - 1] == a[k];
> }
> 
> int func2(int *a, int j) {
>   int k = j - 1;
>   return a[k] == a[j-1];
> }
> 
> ------------------------------------
> Assembly comparison:
> 
> Before this commit:
> 
> func1(int*, int):
>         movslq  %esi, %rax
>         subl    $1, %esi
>         movslq  %esi, %rsi
>         movl    (%rdi,%rsi,4), %edx
>         cmpl    %edx, -4(%rdi,%rax,4)
>         sete    %al
>         movzbl  %al, %eax
>         ret
> func2(int*, int):
>         movl    $1, %eax
>         ret
> 
> After the commit:
> 
> func1(int*, int):
>         movl    $1, %eax
>         ret
> func2(int*, int):
>         leal    -1(%rsi), %eax
>         movslq  %esi, %rsi
>         cltq
>         movl    -4(%rdi,%rsi,4), %edx
>         cmpl    %edx, (%rdi,%rax,4)
>         sete    %al
>         movzbl  %al, %eax
>         ret
> ----------------------------------------
> 
> The patch fixes func1 but breaks func2. These are semantically identical
> functions - the optimization result should not depend on operand order.

The reason why this depends on order is that for func1 during VN the
special handling of sign-changed add triggers in one but not the other
direction, aka we can prove equality of the 2nd to the 1st for

  _1 = (long unsigned int) j_11(D);
  _16 = _1 + 18446744073709551615;

  k_12 = j_11(D) + -1;
  _6 = (long unsigned int) k_12;

but not for

  k_12 = j_11(D) + -1;
  _1 = (long unsigned int) k_12;

  _5 = (long unsigned int) j_11(D);
  _16 = _5 + 18446744073709551615;

that's a matter of implementing the reverse in visit_nary_op.  I have splitted
out PR124545.


> Additionally, this transformation prevents efficient SIB addressing on x86.
> I tried to implement backend compensation, but only partial recovery is 
> possible in some complex benchmarks.
> 
> Should we revert this patch? Or alternatively, I can disable this conversion 
> for x86.

Reply via email to