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] (NFC) Fix test after loosening requirements for register re… #71634

Merged
merged 1 commit into from
Nov 8, 2023

Conversation

nocchijiang
Copy link
Contributor

…naming

The landing of https://reviews.llvm.org/D88663 renders the existing stp-opt-with-renaming-undef-assert test useless because the picked register for renaming becomes q0 instead of q16.

…naming

The landing of https://reviews.llvm.org/D88663 renders the existing
stp-opt-with-renaming-undef-assert test useless because the picked
register for renaming becomes q0 instead of q16.
@llvmbot
Copy link
Collaborator

llvmbot commented Nov 8, 2023

@llvm/pr-subscribers-backend-aarch64

Author: Zhaoxuan Jiang (nocchijiang)

Changes

…naming

The landing of https://reviews.llvm.org/D88663 renders the existing stp-opt-with-renaming-undef-assert test useless because the picked register for renaming becomes q0 instead of q16.


Full diff: https://github.com/llvm/llvm-project/pull/71634.diff

1 Files Affected:

  • (modified) llvm/test/CodeGen/AArch64/stp-opt-with-renaming-undef-assert.mir (+4-4)
diff --git a/llvm/test/CodeGen/AArch64/stp-opt-with-renaming-undef-assert.mir b/llvm/test/CodeGen/AArch64/stp-opt-with-renaming-undef-assert.mir
index 17ce26aa3651466..3030a8456bb607a 100644
--- a/llvm/test/CodeGen/AArch64/stp-opt-with-renaming-undef-assert.mir
+++ b/llvm/test/CodeGen/AArch64/stp-opt-with-renaming-undef-assert.mir
@@ -4,8 +4,8 @@
 # assertion:
 # "Rename register used between paired instruction, trashing the content".
 #
-# The assertion was previously triggered because q16 is picked as renamable
-# register, which overlap with renamable undef d16 used by ZIP2 instruction.
+# The assertion was previously triggered because q0 is picked as renamable
+# register, which overlap with renamable undef d0 used by ZIP2 instruction.
 # However, the content of an undef register is not used in meaningful way,
 # aarch64 load store optimizer should not throw an assertion if a renamable
 # register picked overlap with a renamable undef register.
@@ -20,7 +20,7 @@
 # CHECK-NEXT: $q0 = EXTv16i8 renamable $q23, renamable $q23, 8
 # CHECK-NEXT: renamable $q20 = EXTv16i8 renamable $q14, renamable $q14, 8
 # CHECK-NEXT: STRQui killed renamable $q20, $sp, 4 :: (store (s128))
-# CHECK-NEXT: renamable $d6 = ZIP2v8i8 renamable $d23, undef renamable $d16
+# CHECK-NEXT: renamable $d6 = ZIP2v8i8 renamable $d23, undef renamable $d0
 # CHECK-NEXT: STRDui killed renamable $d6, $sp, 11 :: (store (s64))
 # CHECK-NEXT: renamable $q6 = EXTv16i8 renamable $q13, renamable $q13, 8
 # CHECK-NEXT: STPQi killed renamable $q6, killed $q0, $sp, 6 :: (store (s128))
@@ -46,7 +46,7 @@ body:             |
     STRQui killed renamable $q20, $sp, 7 :: (store (s128))
     renamable $q20 = EXTv16i8 renamable $q14, renamable $q14, 8
     STRQui killed renamable $q20, $sp, 4 :: (store (s128))
-    renamable $d6 = ZIP2v8i8 renamable $d23, undef renamable $d16
+    renamable $d6 = ZIP2v8i8 renamable $d23, undef renamable $d0
     STRDui killed renamable $d6, $sp, 11 :: (store (s64))
     renamable $q6 = EXTv16i8 renamable $q13, renamable $q13, 8
     STRQui killed renamable $q6, $sp, 6 :: (store (s128))

@nocchijiang
Copy link
Contributor Author

I was preparing to make a similar patch and happily found that @c-rhodes has just landed one with basically the same idea behind. However I noticed that one of the tests was not updated correctly, hence this tiny NFC patch.

Since I don't have commit access, please help me add reviewers and merge the PR if it looks good to you. Thanks!

Copy link
Collaborator

@c-rhodes c-rhodes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fix! LGTM

@c-rhodes c-rhodes merged commit 76b53a0 into llvm:main Nov 8, 2023
3 of 4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants