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.

Sign up for GitHub

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

Jump to bottom

[x86] convert xor with MIN_SIGNED_VALUE to add #51609

Closed
rotateright opened this issue Oct 22, 2021 · 9 comments
Closed

[x86] convert xor with MIN_SIGNED_VALUE to add #51609

rotateright opened this issue Oct 22, 2021 · 9 comments
Labels
bugzilla Issues migrated from bugzilla llvm:optimizations

Comments

@rotateright
Copy link
Contributor

rotateright commented Oct 22, 2021 •
edited by RKSimon

Bugzilla Link 52267
Version trunk
OS All
CC @topperc,@RKSimon,@phoebewang,@rotateright

Extended Description

Based on discussion in https://reviews.llvm.org/D112085 :

define i32 @add_smin(i32 %x) {
  %r = add i32 %x, 2147483648
  ret i32 %r
}

define i32 @xor_smin(i32 %x) {
  %r = xor i32 %x, 2147483648
  ret i32 %r
}

These are logically equivalent:
https://alive2.llvm.org/ce/z/qV46E2

...and instcombine chooses 'xor' for better analysis in IR. But x86 (probably unlike most targets), may benefit from converting it back to 'add' (for legal integer types) because that can be lowered as LEA:

  movl	%edi, %eax
  xorl	$-2147483648, %eax              ## imm = 0x80000000

vs.

  leal	-2147483648(%rdi), %eax
@topperc
Copy link
Collaborator

topperc commented Oct 22, 2021

Can we just add an isel pattern?

@RKSimon
Copy link
Collaborator

RKSimon commented Oct 22, 2021

Can we just add an isel pattern?

And to X86DAGToDAGISel::matchAddressRecursively as well maybe?

@RKSimon
Copy link
Collaborator

RKSimon commented Oct 22, 2021

Can we just add an isel pattern?

And to X86DAGToDAGISel::matchAddressRecursively as well maybe?

Although a INT_MIN address offset is pretty rare.....

@rotateright
Copy link
Contributor Author

rotateright commented Oct 22, 2021 •
edited by RKSimon

Depends if we think there's potential for secondary combines?

define i32 @src(i32 %x) {
  %x4 = shl i32 %x, 2
  %r = xor i32 %x4, 2147483648
  ret i32 %r
}

define i32 @tgt(i32 %x) {
  %x4 = shl i32 %x, 2
  %r = add i32 %x4, 2147483648
  ret i32 %r
}

That's

	leal	(,%rdi,4), %eax
	xorl	$-2147483648, %eax              ## imm = 0x80000000

or:

	leal	-2147483648(,%rdi,4), %eax

@llvmbot llvmbot transferred this issue from llvm/llvm-bugzilla-archive Dec 11, 2021
RKSimon added a commit that referenced this issue Mar 30, 2022
@RKSimon
[X86] Add test coverage based off Issue #51609
e000dbc
@RKSimon
Copy link
Collaborator

RKSimon commented Mar 30, 2022

Before adding X86 specific handling, I've proposed adding generic AddLike support in https://reviews.llvm.org/D122754

@RKSimon
Copy link
Collaborator

RKSimon commented Mar 31, 2022

matchAddressRecursively support - https://reviews.llvm.org/D122815

@RKSimon
Copy link
Collaborator

RKSimon commented Apr 2, 2022

matchAddressRecursively support - https://reviews.llvm.org/D122815

Landed at c64f37f

@RKSimon RKSimon changed the title [x86] convert xor with SMIN to add [x86] convert xor with MIN_SIGNED_VALUE to add Apr 2, 2022
@RKSimon
Copy link
Collaborator

RKSimon commented Apr 6, 2022

[X86] Add XOR(X, MIN_SIGNED_VALUE) -> ADD(X, MIN_SIGNED_VALUE) isel patterns (PR52267)
https://reviews.llvm.org/D123043

Landed at ffe0cc8

@RKSimon
Copy link
Collaborator

RKSimon commented Apr 6, 2022

Before adding X86 specific handling, I've proposed adding generic AddLike support in https://reviews.llvm.org/D122754

Landed at 3369e47

@RKSimon RKSimon closed this as completed Apr 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bugzilla Issues migrated from bugzilla llvm:optimizations
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@RKSimon @EugeneZelenko @rotateright @topperc