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

or+and miscompile with global isel on arm64 #55284

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

or+and miscompile with global isel on arm64 #55284

regehr opened this issue May 5, 2022 · 4 comments

Comments

@regehr
Copy link
Contributor

regehr commented May 5, 2022

we're seeing a miscompile of this using top of tree with global isel:

define i32 @f3(i32 %0) {
  %2 = or i32 %0, 65536
  %3 = and i32 1520220788, %2
  ret i32 %3
}

the output is:

	.section	__TEXT,__text,regular,pure_instructions
	.build_version macos, 12, 0
	.globl	_f3                             ; -- Begin function f3
	.p2align	2
_f3:                                    ; @f3
	.cfi_startproc
; %bb.0:
	orr	w8, w0, #0x10000
	and	w0, w0, w8
	ret
	.cfi_endproc
                                        ; -- End function
.subsections_via_symbols

here's a driver showing the issue, and that the default codegen gives the right answer:

Johns-MacBook-Pro:~ regehr$ cat foo.c
#include <stdio.h>

unsigned f3(unsigned);

int main(void) {
  printf("%u\n", f3(1));
  return 0;
}

Johns-MacBook-Pro:~ regehr$ ~/llvm-project/for-alive/bin/llc foo.ll -global-isel 
Johns-MacBook-Pro:~ regehr$ clang foo.c foo.s && ./a.out
1
Johns-MacBook-Pro:~ regehr$ ~/llvm-project/for-alive/bin/llc foo.ll
Johns-MacBook-Pro:~ regehr$ clang foo.c foo.s && ./a.out
0
Johns-MacBook-Pro:~ regehr$ 

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

@llvmbot
Copy link
Member

llvmbot commented May 5, 2022

@llvm/issue-subscribers-backend-aarch64

@regehr regehr changed the title miscompile with global isel on arm64 or+and miscompile with global isel on arm64 May 5, 2022
@jroelofs
Copy link
Contributor

Sounds like: b046eb1

@jroelofs
Copy link
Contributor

jroelofs commented May 12, 2022

Yep, this needs to choose the operand more carefully:

   MatchInfo = [=, &MI](MachineIRBuilder &B) {
     Observer.changingInstr(MI);
     MI.getOperand(1).setReg(Src);
     Observer.changedInstr(MI);
   };
   return true;

b046eb1#diff-8c5f6b76b226d2087ec6bd0a7c643fba246650ab64f0b9ef7b4f4bda3fd7f70eR4034-R4039

https://llvm.godbolt.org/z/fxh463vMW

@jroelofs jroelofs self-assigned this May 12, 2022
@jroelofs
Copy link
Contributor

https://reviews.llvm.org/D125472

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