Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

miscompile from arm64 backend with (icmp ult (sub -6, -8) 3) #55490

Closed
regehr opened this issue May 15, 2022 · 8 comments
Closed

miscompile from arm64 backend with (icmp ult (sub -6, -8) 3) #55490

regehr opened this issue May 15, 2022 · 8 comments

Comments

@regehr
Copy link
Contributor

regehr commented May 15, 2022

here's a function that needs to return true:

define i1 @f() {
  %1 = sub i4 -6, -8
  %2 = icmp ult i4 %1, 3
  ret i1 %2
}

however, since the 14 release the arm64 backend got a regression causing it to return false:

regehr@john-home:~/tmp$ ~/clang+llvm-14.0.0-x86_64-linux-gnu-ubuntu-18.04/bin/llc foo.ll -o - -march=arm64
	.text
	.file	"foo.ll"
	.globl	f                               // -- Begin function f
	.p2align	2
	.type	f,@function
f:                                      // @f
	.cfi_startproc
// %bb.0:
	mov	w0, #1
	ret
.Lfunc_end0:
	.size	f, .Lfunc_end0-f
	.cfi_endproc
                                        // -- End function
	.section	".note.GNU-stack","",@progbits
regehr@john-home:~/tmp$ ~/llvm-project/for-alive/bin/llc foo.ll -o - -march=arm64
	.text
	.file	"foo.ll"
	.globl	f                               // -- Begin function f
	.p2align	2
	.type	f,@function
f:                                      // @f
	.cfi_startproc
// %bb.0:
	mov	w0, wzr
	ret
.Lfunc_end0:
	.size	f, .Lfunc_end0-f
	.cfi_endproc
                                        // -- End function
	.section	".note.GNU-stack","",@progbits
regehr@john-home:~/tmp$ 

cc @ornata @nunoplopes @ryan-berger @nbushehri @zhengyang92 @aqjune @Hatsunespica

@llvmbot
Copy link
Member

llvmbot commented May 15, 2022

@llvm/issue-subscribers-backend-aarch64

@topperc
Copy link
Collaborator

topperc commented May 15, 2022

Is this the same as #55342?

@regehr
Copy link
Contributor Author

regehr commented May 15, 2022

Is this the same as #55342?

well it sure looks related but no, this one is still wrong using an LLVM that has your patch to that one

@regehr
Copy link
Contributor Author

regehr commented May 15, 2022

if y'all prefer, we can sit on these ones that are highly non-canonical, but I feel like it's hard to prove that there aren't canonical triggers for this issue

@topperc
Copy link
Collaborator

topperc commented May 15, 2022

Looks like a bug in the same code as 55342. When the safewrap instruction is a subtract, instead of sign extending the constant, we need to negate it, sign extend it, then negate it again, I think. I think the sign extend by itself is only correct for add.

@topperc
Copy link
Collaborator

topperc commented May 16, 2022

Candidate patch https://reviews.llvm.org/D125653

@regehr
Copy link
Contributor Author

regehr commented May 16, 2022

ok here's a trigger for this issue that, while not being canonical, at least doesn't contain a trivial fold opportunity

define i1 @f(i4 %0) {
  %2 = sub i4 %0, 6
  %3 = icmp ult i4 -5, %2
  ret i1 %3
}

@fhahn fhahn changed the title miscompile from arm64 backend miscompile from arm64 backend with (icmp ult (sub -6, -8) 3) May 17, 2022
@topperc
Copy link
Collaborator

topperc commented May 20, 2022

Fixed by 8d3894f

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants