-
Notifications
You must be signed in to change notification settings - Fork 15.3k
[UTC] Update AMDGPU asm regexp for private functions #166169
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
Conversation
Since llvm#163011 changed AMDGPU to use ELF mangling, the regexp failed to match private functions because of the inconsistent presence/absence of the .L prefix on the first line of the function e.g.: .Lfoo: ; @foo
|
@llvm/pr-subscribers-testing-tools Author: Jay Foad (jayfoad) ChangesSince #163011 changed AMDGPU to use ELF mangling, the regexp failed to .Lfoo: ; @foo Full diff: https://github.com/llvm/llvm-project/pull/166169.diff 2 Files Affected:
diff --git a/llvm/test/CodeGen/AMDGPU/private-function.ll b/llvm/test/CodeGen/AMDGPU/private-function.ll
new file mode 100644
index 0000000000000..8eefc9dfc5d7e
--- /dev/null
+++ b/llvm/test/CodeGen/AMDGPU/private-function.ll
@@ -0,0 +1,16 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --version 6
+; RUN: llc -mtriple=amdgcn -mcpu=gfx1200 < %s | FileCheck %s
+
+define private void @foo() {
+; CHECK-LABEL: foo:
+; CHECK: ; %bb.0:
+; CHECK-NEXT: s_wait_loadcnt_dscnt 0x0
+; CHECK-NEXT: s_wait_expcnt 0x0
+; CHECK-NEXT: s_wait_samplecnt 0x0
+; CHECK-NEXT: s_wait_bvhcnt 0x0
+; CHECK-NEXT: s_wait_kmcnt 0x0
+; CHECK-NEXT: s_setpc_b64 s[30:31]
+ ret void
+}
+
+@var = global ptr @foo
diff --git a/llvm/utils/UpdateTestChecks/asm.py b/llvm/utils/UpdateTestChecks/asm.py
index 469e27facedb0..036e030eb61ba 100644
--- a/llvm/utils/UpdateTestChecks/asm.py
+++ b/llvm/utils/UpdateTestChecks/asm.py
@@ -51,9 +51,9 @@ class string:
)
ASM_FUNCTION_AMDGPU_RE = re.compile(
- r"\.type\s+_?(?P<func>[^,\n]+),@function\n"
+ r"\.type\s+(_|\.L)?(?P<func>[^,\n]+),@function\n"
r"(^\s*\.amdgpu_hsa_kernel (?P=func)\n)?"
- r'^_?(?P=func):(?:[ \t]*;+[ \t]*@"?(?P=func)"?)?\n'
+ r'^(_|\.L)?(?P=func):(?:[ \t]*;+[ \t]*@"?(?P=func)"?)?\n'
r"(?P<body>.*?)\n" # (body of the function)
# This list is incomplete
r"^\s*(\.Lfunc_end[0-9]+:\n|\.section)",
|
|
@llvm/pr-subscribers-backend-amdgpu Author: Jay Foad (jayfoad) ChangesSince #163011 changed AMDGPU to use ELF mangling, the regexp failed to .Lfoo: ; @foo Full diff: https://github.com/llvm/llvm-project/pull/166169.diff 2 Files Affected:
diff --git a/llvm/test/CodeGen/AMDGPU/private-function.ll b/llvm/test/CodeGen/AMDGPU/private-function.ll
new file mode 100644
index 0000000000000..8eefc9dfc5d7e
--- /dev/null
+++ b/llvm/test/CodeGen/AMDGPU/private-function.ll
@@ -0,0 +1,16 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --version 6
+; RUN: llc -mtriple=amdgcn -mcpu=gfx1200 < %s | FileCheck %s
+
+define private void @foo() {
+; CHECK-LABEL: foo:
+; CHECK: ; %bb.0:
+; CHECK-NEXT: s_wait_loadcnt_dscnt 0x0
+; CHECK-NEXT: s_wait_expcnt 0x0
+; CHECK-NEXT: s_wait_samplecnt 0x0
+; CHECK-NEXT: s_wait_bvhcnt 0x0
+; CHECK-NEXT: s_wait_kmcnt 0x0
+; CHECK-NEXT: s_setpc_b64 s[30:31]
+ ret void
+}
+
+@var = global ptr @foo
diff --git a/llvm/utils/UpdateTestChecks/asm.py b/llvm/utils/UpdateTestChecks/asm.py
index 469e27facedb0..036e030eb61ba 100644
--- a/llvm/utils/UpdateTestChecks/asm.py
+++ b/llvm/utils/UpdateTestChecks/asm.py
@@ -51,9 +51,9 @@ class string:
)
ASM_FUNCTION_AMDGPU_RE = re.compile(
- r"\.type\s+_?(?P<func>[^,\n]+),@function\n"
+ r"\.type\s+(_|\.L)?(?P<func>[^,\n]+),@function\n"
r"(^\s*\.amdgpu_hsa_kernel (?P=func)\n)?"
- r'^_?(?P=func):(?:[ \t]*;+[ \t]*@"?(?P=func)"?)?\n'
+ r'^(_|\.L)?(?P=func):(?:[ \t]*;+[ \t]*@"?(?P=func)"?)?\n'
r"(?P<body>.*?)\n" # (body of the function)
# This list is incomplete
r"^\s*(\.Lfunc_end[0-9]+:\n|\.section)",
|
|
|
||
| ASM_FUNCTION_AMDGPU_RE = re.compile( | ||
| r"\.type\s+_?(?P<func>[^,\n]+),@function\n" | ||
| r"\.type\s+(_|\.L)?(?P<func>[^,\n]+),@function\n" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure if we need to match an underscore _ prefix. Do we ever generate that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it used in any of the existing tests?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think so (based on regenerating all tests in test/CodeGen/AMDGPU/, then removing the underscore handling here, then regenerating them all a second time).
| ASM_FUNCTION_AMDGPU_RE = re.compile( | ||
| r"\.type\s+_?(?P<func>[^,\n]+),@function\n" | ||
| r"\.type\s+(_|\.L)?(?P<func>[^,\n]+),@function\n" | ||
| r"(^\s*\.amdgpu_hsa_kernel (?P=func)\n)?" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure if this needs to tolerate a prefix before the function name.
Since #163011 changed AMDGPU to use ELF mangling, the regexp failed to
match private functions because of the inconsistent presence/absence of
the .L prefix on the first line of the function e.g.: