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

AMDGPU silently converts incorrect physical register asm constraint to virtual register #123208

Closed
arsenm opened this issue Jan 16, 2025 · 2 comments · Fixed by #123590
Closed

Comments

@arsenm
Copy link
Contributor

arsenm commented Jan 16, 2025

; RUN: llc -mtriple=amdgcn-amd-amdhsa -mcpu=gfx940 < %s

define void @invalid_sgpr(<2 x i32> inreg %arg0) {
  call void asm sideeffect "; use $0", "{s[1:2]}"(<2 x i32> %arg0)
  ret void
}

s[1:2] is not a valid SGPR reference as 64-bit SGPRs require even alignment. This is silently accepted, and appears to be treated as a virtual register constraint. In -stop-after=finalize-isel, I see:

    %10:sreg_64 = COPY %11
    INLINEASM &"; use $0", 1 /* sideeffect attdialect */, 3997705 /* reguse:SReg_64 */, %10
@llvmbot
Copy link
Member

llvmbot commented Jan 16, 2025

@llvm/issue-subscribers-backend-amdgpu

Author: Matt Arsenault (arsenm)

``` ; RUN: llc -mtriple=amdgcn-amd-amdhsa -mcpu=gfx940 < %s

define void @invalid_sgpr(<2 x i32> inreg %arg0) {
call void asm sideeffect "; use $0", "{s[1:2]}"(<2 x i32> %arg0)
ret void
}


s[1:2] is not a valid SGPR reference as 64-bit SGPRs require even alignment. This is silently accepted, and appears to be treated as a virtual register constraint. In -stop-after=finalize-isel, I see:

%10:sreg_64 = COPY %11
INLINEASM &amp;"; use $0", 1 /* sideeffect attdialect */, 3997705 /* reguse:SReg_64 */, %10
</details>

@ritter-x2a ritter-x2a self-assigned this Jan 20, 2025
ritter-x2a added a commit to ritter-x2a/llvm-project that referenced this issue Jan 20, 2025
The indices of SGPR register pairs need to be 2-aligned and SGPR
quadruplets need to be 4-aligned. With this patch, we report an error
when inline asm register constraints specify a misaligned register
index, instead of silently dropping the specified index.

Fixes llvm#123208
@ritter-x2a
Copy link
Member

#123590 has a patch for this problem.

ritter-x2a added a commit that referenced this issue Jan 20, 2025
The indices of SGPR register pairs need to be 2-aligned and SGPR
quadruplets need to be 4-aligned. With this patch, we report an error
when inline asm register constraints specify a misaligned register
index, instead of silently dropping the specified index.

Fixes #123208

---------

Co-authored-by: Matt Arsenault <[email protected]>
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.

3 participants