Quuxplusone / LLVMBugzillaTest

0 stars 0 forks source link

Reuse flags from xor; remove cmp #41984

Open Quuxplusone opened 5 years ago

Quuxplusone commented 5 years ago
Bugzilla Link PR43014
Status REOPENED
Importance P enhancement
Reported by David Bolvansky (david.bolvansky@gmail.com)
Reported on 2019-08-15 14:55:01 -0700
Last modified on 2020-08-08 11:37:11 -0700
Version trunk
Hardware PC Linux
CC craig.topper@gmail.com, kamleshbhalui@gmail.com, llvm-bugs@lists.llvm.org, llvm-dev@redking.me.uk, spatel+llvm@rotateright.com
Fixed by commit(s)
Attachments
Blocks
Blocked by
See also
#include <stdlib.h>
#include <stdio.h>
int p;

__attribute_noinline__ int foo(int x, int y) {
   p = x ^ y;
   if (x - y == 0) abort();
   return 0;
}

int main(int argc, char**argv) {
   foo(argc, atoi(argv[1]));
   printf("p = %d\n", p);
}

foo:                                    # @foo
    .cfi_startproc
# %bb.0:                                # %entry
    pushq   %rax
    .cfi_def_cfa_offset 16
    movl    %esi, %eax
    xorl    %edi, %eax
    movl    %eax, p(%rip)
    cmpl    %esi, %edi
    je  .LBB0_2

XOR sets ZF flag so.. I tried to remove cmp and compiled hand modified asm..

foo:                                    # @foo
    .cfi_startproc
# %bb.0:                                # %entry
    pushq   %rax
    .cfi_def_cfa_offset 16
    movl    %esi, %eax
    xorl    %edi, %eax
    movl    %eax, p(%rip)
    jz  .LBB0_2

and test case still  works as expected.
Quuxplusone commented 5 years ago

can someone confirm it?

Quuxplusone commented 5 years ago
This feels sort of specific. It only happens if you mix xor and sub.

__attribute_noinline__ int foo(int x, int y) {
   p = x ^ y;
   if (p == 0) abort();
   return 0;
}

Or

__attribute_noinline__ int foo(int x, int y) {
   p = x ^ y;
   if (x ^ y == 0) abort();
   return 0;
}

We'll generate the code that's expected. Neither gcc, icc, or MSVC optimize the
original code either.

If we were to fix this we should do it for other targets. There's nothing X86
specific here.
Quuxplusone commented 4 years ago

Should be fixed after 44b260cb0aab387d85e4d59c16fc7b8866264f5e

Quuxplusone commented 4 years ago

Oops I closed the wrong bug