Open DimitryAndric opened 5 years ago
Another note is about the minimized test case:
unsigned f(const unsigned char *input) { unsigned crc = ~0; asm("crc32b (%[in]), %[crc]" : "+c"(crc) : [crc] "c"(crc), [in] "r"(input)); return ~crc; }
compiled with gcc (10.3.0) -O1 -S results in:
f: .LFB0: .cfi_startproc movl $-1, %ecx
crc32b (%rdi), %ecx
movl %ecx, %eax
notl %eax
ret
and compiled with clang (12.0.0, disabled assertions) -O1 -S results in:
f: # @f .cfi_startproc
movl $-1, %ecx
movl $-1, %ecx # <<< huh?
#APP
crc32b (%rdi), %ecx
#NO_APP
movl %ecx, %eax
notl %eax
retq
So for some weird reason, clang duplicates the movl $-1, %ecx
instruction? Maybe this is what the assertion originall was trying to guard against?
Note that the whole assertion block:
// First, verify that we don't have a use of "a" in the instruction
// (a = b + a for example) because our transformation will not
// work. This should never occur because we are in SSA form.
for (unsigned i = 0; i != MI->getNumOperands(); ++i)
assert(i == DstIdx ||
!MI->getOperand(i).isReg() ||
MI->getOperand(i).getReg() != RegA);
was added a very long time ago by Alkis Evlogimenos in r10682: https://github.com/llvm/llvm-project/commit/08c5311729b684619e307b561fbecc0095e85bfe:
Date: Mon Jan 5 02:25:45 2004 +0000
Currently we cannot handle two-address instructions of the form:
A = B op C where A == C, but this cannot really occur in practice
because of SSA form. Add an assert to check that just to be safe.
llvm-svn: 10682
So my question to Alkis (and Chris, since he moved some stuff in this pass around in 2004 too): is this "currently we cannot handle" comment still applicable at all? Since if you compile LLVM with assertions turned off, this whole block is now simply skipped, and the resulting assembly appears to work Just Fine? :)
A possible duplicate.
$ clang-trunk -v clang version 13.0.0 (https://github.com/llvm/llvm-project.git 6b0d266036f73f5ee9556d211a7d0946091ff3b2) Target: x86_64-unknown-linux-gnu Thread model: posix InstalledDir: /home/cnsun/usr/bin Found candidate GCC installation: /usr/lib/gcc/x86_64-linux-gnu/10 Found candidate GCC installation: /usr/lib/gcc/x86_64-linux-gnu/8 Found candidate GCC installation: /usr/lib/gcc/x86_64-linux-gnu/9 Selected GCC installation: /usr/lib/gcc/x86_64-linux-gnu/10 Candidate multilib: .;@m64 Candidate multilib: 32;@m32 Candidate multilib: x32;@mx32 Selected multilib: .;@m64
$ cat mutant.c a; fn1() { int b; asm(" # a: %x0, b: %x1" : "+d"(a), "+d"(b)); }
$ clang-trunk mutant.c mutant.c:1:1: warning: type specifier missing, defaults to 'int' [-Wimplicit-int] a; ^ mutant.c:2:1: warning: type specifier missing, defaults to 'int' [-Wimplicit-int] fn1() { ^ mutant.c:5:1: warning: non-void function does not return a value [-Wreturn-type] } ^ clang-13: /tmp/tmp.OKn6kNiY8c-clang-builder/llvm-project/llvm/lib/CodeGen/TwoAddressInstructionPass.cpp:1398: void {anonymous}::TwoAddressInstructionPass::processTiedPairs(llvm::MachineInstr*, {anonymous}::TwoAddressInstructionPass::TiedPairList&, unsigned int&): Assertion `i == DstIdx || !MI->getOperand(i).isReg() || MI->getOperand(i).getReg() != RegA' failed. PLEASE submit a bug report to https://bugs.llvm.org/ and include the crash backtrace, preprocessed source, and associated run script. Stack dump:
clang-13: error: unable to execute command: Aborted (core dumped) clang-13: error: clang frontend command failed due to signal (use -v to see invocation) clang version 13.0.0 (https://github.com/llvm/llvm-project.git 6b0d266036f73f5ee9556d211a7d0946091ff3b2) Target: x86_64-unknown-linux-gnu Thread model: posix InstalledDir: /home/cnsun/usr/bin clang-13: note: diagnostic msg:
PLEASE ATTACH THE FOLLOWING FILES TO THE BUG REPORT: Preprocessed source(s) and associated run script(s) are located at: clang-13: note: diagnostic msg: /tmp/mutant-187aed.c clang-13: note: diagnostic msg: /tmp/mutant-187aed.sh clang-13: note: diagnostic msg:
Still occurs as of 2020-11-08, with llvmorg-12-init-10988-g43df29e2062:
$ ~/ins/llvmorg-12-init-10988-g43df29e2062/bin/clang -cc1 -triple x86_64-- -S crc32c_sse42_asm-min.c Assertion failed: (i == DstIdx || !MI->getOperand(i).isReg() || MI->getOperand(i).getReg() != RegA), function processTiedPairs, file /home/dim/src/llvm/llvm-project/llvm/lib/CodeGen/TwoAddressInstructionPass.cpp, line 1415. PLEASE submit a bug report to https://bugs.llvm.org/ and include the crash backtrace, preprocessed source, and associated run script. Stack dump:
Abort trap
This reduced case isn't valid code according to gcc. It says ecx should be bound to b for input and output. And the ecx should also be bound to a for input. Clang doesn't diagnose that properly and hits the same error from the larger bug. Is it possible this wasn't reduced correctly?
Possibly. I've used creduce, and it tries to simplify inline asm quite a lot. The original code is full of +c constraints, like:
asm("loopsmall%=: CRC32B (%[in]), %[crc]" : "+c"(crc) : [crc] "c"(crc), [in] "r"(input));
So probably a better minimized case is:
unsigned f(const unsigned char *input) { unsigned crc = ~0; asm("crc32b (%[in]), %[crc]" : "+c"(crc) : [crc] "c"(crc), [in] "r"(input)); return ~crc; }
though gcc 8 requires at least -O1, otherwise it says the constraints are impossible. With -O1 or higher, it produces:
$ gcc8 -O2 -S crc32c_sse42_asm-min-2.c -o - .file "crc32c_sse42_asm-min-2.c" .text .p2align 4,,15 .globl f .type f, @function f: .LFB0: .cfi_startproc movl $-1, %ecx
crc32b (%rdi), %ecx
movl %ecx, %eax
notl %eax
ret
.cfi_endproc
.LFE0: .size f, .-f .ident "GCC: (FreeBSD Ports Collection) 8.3.0" .section .note.GNU-stack,"",@progbits
This reduced case isn't valid code according to gcc. It says ecx should be bound to b for input and output. And the ecx should also be bound to a for input. Clang doesn't diagnose that properly and hits the same error from the larger bug. Is it possible this wasn't reduced correctly?
Still an issue, see #54957
Extended Description
As reported in https://bugs.freebsd.org/234232, building the devel/aws-checksums port (see also https://github.com/awslabs/aws-checksums) results in a clang assertion:
Assertion failed: (i == DstIdx || !MI->getOperand(i).isReg() || MI->getOperand(i).getReg() != RegA), function processTiedPairs, file lib/CodeGen/TwoAddressInstructionPass.cpp, line 1547.
Minimized test case:
/ clang -cc1 -triple x86_64-- -S crc32c_sse42_asm-min.c / int a, b; void c() { asm("" : "+c"(b) : "c"(a)); }