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 by arm64 backend #55342

Closed
regehr opened this issue May 9, 2022 · 4 comments
Closed

miscompile by arm64 backend #55342

regehr opened this issue May 9, 2022 · 4 comments

Comments

@regehr
Copy link
Contributor

regehr commented May 9, 2022

this function should return 1:

define i32 @f() {
  %1 = sub i8 -66, 0
  %2 = icmp ugt i8 -31, %1
  %3 = select i1 %2, i32 1, i32 0
  ret i32 %3
}

however, the arm64 backend (in LLVM 14 and also top of tree) has it returning 0:

regehr@john-home:~$ ~/llvm-project/for-alive/bin/llc foo.ll -o - -march=aarch64
	.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:~$ 

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

@llvmbot
Copy link
Collaborator

llvmbot commented May 9, 2022

@llvm/issue-subscribers-backend-aarch64

@topperc
Copy link
Collaborator

topperc commented May 9, 2022

@sparker-arm looks like the TypePromotion pass.

@topperc
Copy link
Collaborator

topperc commented May 9, 2022

I think PromoteTree shouldn't sext the first operand for SafeWrap instructions. Something like this

diff --git a/llvm/lib/CodeGen/TypePromotion.cpp b/llvm/lib/CodeGen/TypePromotion.cpp
index 48454a5047dd..0caf763c7ff0 100644
--- a/llvm/lib/CodeGen/TypePromotion.cpp
+++ b/llvm/lib/CodeGen/TypePromotion.cpp
@@ -484,7 +484,7 @@ void IRPromoter::PromoteTree() {
         continue;
 
       if (auto *Const = dyn_cast<ConstantInt>(Op)) {
-        Constant *NewConst = SafeWrap.contains(I)
+        Constant *NewConst = (SafeWrap.contains(I) && i == 1)
                                  ? ConstantExpr::getSExt(Const, ExtTy)
                                  : ConstantExpr::getZExt(Const, ExtTy);
         I->setOperand(i, NewConst);

@davemgreen
Copy link
Collaborator

That sounds good to me, I don't think the pass handles non-canonical IR very well at the moment.

I can put together a patch, if you don't have one already.

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

5 participants