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

AArch64 backend incorrectly selecting uabdl #88784

Closed
regehr opened this issue Apr 15, 2024 · 2 comments · Fixed by #89272
Closed

AArch64 backend incorrectly selecting uabdl #88784

regehr opened this issue Apr 15, 2024 · 2 comments · Fixed by #89272

Comments

@regehr
Copy link
Contributor

regehr commented Apr 15, 2024

this function:

define <8 x i16> @f(<8 x i8> %0, <8 x i8> %1, <8 x i16> %2) {
  %4 = zext <8 x i8> %0 to <8 x i16>
  %5 = ashr <8 x i16> %2, <i16 15, i16 15, i16 15, i16 15, i16 15, i16 15, i16 15, i16 15>
  %6 = zext <8 x i8> %1 to <8 x i16>
  %7 = sub <8 x i16> %4, %6
  %8 = add <8 x i16> %5, %7
  %9 = xor <8 x i16> %5, %8
  ret <8 x i16> %9
}

is getting lowered by the AArch64 backend to:

_f: 
	uabdl.8h	v0, v0, v1
	ret

but I don't think that's right. if we call f() like this:

  %x = call <8 x i16> @f(<8 x i8>  <i8 0,  i8 0,  i8 0,  i8 0,  i8 0,  i8 0,  i8 0,  i8 0>,
                         <8 x i8>  <i8 0,  i8 13, i8 0,  i8 0,  i8 0,  i8 0,  i8 0,  i8 0>,
                         <8 x i16> <i16 0, i16 0, i16 0, i16 0, i16 0, i16 0, i16 0,i16 0>)

then we should end up with -13 in lane 1. however, the uabdl puts 13 into that lane.

@llvmbot
Copy link
Collaborator

llvmbot commented Apr 15, 2024

@llvm/issue-subscribers-backend-aarch64

Author: John Regehr (regehr)

this function: ```llvm define <8 x i16> @f(<8 x i8> %0, <8 x i8> %1, <8 x i16> %2) { %4 = zext <8 x i8> %0 to <8 x i16> %5 = ashr <8 x i16> %2, <i16 15, i16 15, i16 15, i16 15, i16 15, i16 15, i16 15, i16 15> %6 = zext <8 x i8> %1 to <8 x i16> %7 = sub <8 x i16> %4, %6 %8 = add <8 x i16> %5, %7 %9 = xor <8 x i16> %5, %8 ret <8 x i16> %9 } ``` is getting lowered by the AArch64 backend to: ``` _f: uabdl.8h v0, v0, v1 ret ``` but I don't think that's right. if we call f() like this: ```llvm %x = call <8 x i16> @f(<8 x i8> <i8 0, i8 0, i8 0, i8 0, i8 0, i8 0, i8 0, i8 0>, <8 x i8> <i8 0, i8 13, i8 0, i8 0, i8 0, i8 0, i8 0, i8 0>, <8 x i16> <i16 0, i16 0, i16 0, i16 0, i16 0, i16 0, i16 0,i16 0>) ``` then we should end up with -13 in lane 1. however, the uabdl puts 13 into that lane.

davemgreen added a commit to davemgreen/llvm-project that referenced this issue Apr 18, 2024
These were added in https://reviews.llvm.org/D14208, which look like they
attempt to detect abs from xor+add+ashr. They do not appear to be detecting the
correct value for the src input though, which I think is intended to be the
sub(zext, zext) part of the pattern.  We have pattens from abs now, so the old
invalid patterns can be removed.

Fixes llvm#88784
@davemgreen
Copy link
Collaborator

It looks like there are some very old patterns that were not validating where the inputs were coming from. I have removed them in #89272. Thanks for the report.

davemgreen added a commit that referenced this issue Apr 19, 2024
These were added in https://reviews.llvm.org/D14208, which look like
they attempt to detect abs from xor+add+ashr. They do not appear to be
detecting the correct value for the src input though, which I think is
intended to be the sub(zext, zext) part of the pattern. We have pattens
from abs now, so the old invalid patterns can be removed.

Fixes #88784
RSilicon pushed a commit to RSilicon/llvm-project that referenced this issue Apr 19, 2024
These were added in https://reviews.llvm.org/D14208, which look like
they attempt to detect abs from xor+add+ashr. They do not appear to be
detecting the correct value for the src input though, which I think is
intended to be the sub(zext, zext) part of the pattern. We have pattens
from abs now, so the old invalid patterns can be removed.

Fixes llvm#88784

(cherry picked from commit 851462f)
aniplcc pushed a commit to aniplcc/llvm-project that referenced this issue Apr 21, 2024
These were added in https://reviews.llvm.org/D14208, which look like
they attempt to detect abs from xor+add+ashr. They do not appear to be
detecting the correct value for the src input though, which I think is
intended to be the sub(zext, zext) part of the pattern. We have pattens
from abs now, so the old invalid patterns can be removed.

Fixes llvm#88784
RSilicon pushed a commit to RSilicon/llvm-project that referenced this issue Apr 23, 2024
These were added in https://reviews.llvm.org/D14208, which look like
they attempt to detect abs from xor+add+ashr. They do not appear to be
detecting the correct value for the src input though, which I think is
intended to be the sub(zext, zext) part of the pattern. We have pattens
from abs now, so the old invalid patterns can be removed.

Fixes llvm#88784

(cherry picked from commit 851462f)
tstellar pushed a commit to RSilicon/llvm-project that referenced this issue Apr 25, 2024
These were added in https://reviews.llvm.org/D14208, which look like
they attempt to detect abs from xor+add+ashr. They do not appear to be
detecting the correct value for the src input though, which I think is
intended to be the sub(zext, zext) part of the pattern. We have pattens
from abs now, so the old invalid patterns can be removed.

Fixes llvm#88784

(cherry picked from commit 851462f)
tstellar pushed a commit to RSilicon/llvm-project that referenced this issue Apr 30, 2024
These were added in https://reviews.llvm.org/D14208, which look like
they attempt to detect abs from xor+add+ashr. They do not appear to be
detecting the correct value for the src input though, which I think is
intended to be the sub(zext, zext) part of the pattern. We have pattens
from abs now, so the old invalid patterns can be removed.

Fixes llvm#88784

(cherry picked from commit 851462f)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants