Skip to content

[AMDGPU][GlobalISel]selectImpl() may create illegal copy from VGPR to SGPR #62035

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

Open
chenzheng1030 opened this issue Apr 10, 2023 · 6 comments

Comments

@chenzheng1030
Copy link
Collaborator

This is found during investigation of issue #61468

After importing more td patterns to global MatchTable, for example the patterns in https://reviews.llvm.org/D147780, selectImpl() will create illegal copy.

For now in https://reviews.llvm.org/D147780, this issue is workaround by not using selectImpl() to do the selection. But maybe a fix in selectImpl() should be made to avoid such issue.

@llvmbot
Copy link
Member

llvmbot commented Apr 10, 2023

@llvm/issue-subscribers-backend-amdgpu

@nickleus27
Copy link
Contributor

I am interested in this issue. Can someone assign to me.

@arsenm
Copy link
Contributor

arsenm commented Mar 4, 2024

There's been a number of changes to boolean handling since this was filed. cc @petar-avramovic

@petar-avramovic
Copy link
Collaborator

Is there a .ll test for this?
This probably has to be fixed before instruction-select, maybe something like: #68858
If not and we want to select it intentionally as vgpr to sgpr copy we need to make use of uniformity info analysis first in #73684

@chenzheng1030
Copy link
Collaborator Author

chenzheng1030 commented Jul 9, 2024

Sorry for the delay. Totally lost track for this issue.

To reproduce the bug, try following change and run ninja check-llvm:

diff --git a/llvm/lib/Target/AMDGPU/AMDGPUInstructionSelector.cpp b/llvm/lib/Target/AMDGPU/AMDGPUInstructionSelector.cpp
index dcb0f47973c4..9be4b6744cc1 100644
--- a/llvm/lib/Target/AMDGPU/AMDGPUInstructionSelector.cpp
+++ b/llvm/lib/Target/AMDGPU/AMDGPUInstructionSelector.cpp
@@ -3609,8 +3609,9 @@ bool AMDGPUInstructionSelector::select(MachineInstr &I) {
     // This is a workaround. For extension from type i1, `selectImpl()` uses
     // patterns from TD file and generates an illegal VGPR to SGPR COPY as type
     // i1 can only be hold in a SGPR class.
-    if (MRI->getType(I.getOperand(1).getReg()) != LLT::scalar(1) &&
-        selectImpl(I, *CoverageInfo))
+    //if (MRI->getType(I.getOperand(1).getReg()) != LLT::scalar(1) &&
+    //    selectImpl(I, *CoverageInfo))
+    if (selectImpl(I, *CoverageInfo))
       return true;
     return selectG_SZA_EXT(I);
   case TargetOpcode::G_FPEXT:
ninja check-llvm -j128

Failed Tests (9):
  LLVM :: CodeGen/AMDGPU/GlobalISel/divergence-divergent-i1-phis-no-lane-mask-merging.ll
  LLVM :: CodeGen/AMDGPU/GlobalISel/divergence-temporal-divergent-i1.ll
  LLVM :: CodeGen/AMDGPU/GlobalISel/inst-select-anyext.mir
  LLVM :: CodeGen/AMDGPU/GlobalISel/inst-select-freeze.mir
  LLVM :: CodeGen/AMDGPU/GlobalISel/inst-select-icmp.s64.mir
  LLVM :: CodeGen/AMDGPU/GlobalISel/inst-select-sext.mir
  LLVM :: CodeGen/AMDGPU/GlobalISel/inst-select-zext.mir
  LLVM :: CodeGen/AMDGPU/div_i128.ll
  LLVM :: CodeGen/AMDGPU/div_v2i128.ll

an error:

FileCheck -check-prefix=GFX10 llvm-project/llvm/test/CodeGen/AMDGPU/GlobalISel/divergence-temporal-divergent-i1.ll
error: <unknown>:0:0: in function temporal_divergent_i1_phi void (float, ptr): illegal VGPR to SGPR copy

error: <unknown>:0:0: in function temporal_divergent_i1_phi void (float, ptr): illegal VGPR to SGPR copy

error: <unknown>:0:0: in function temporal_divergent_i1_non_phi void (float, ptr): illegal VGPR to SGPR copy

error: <unknown>:0:0: in function temporal_divergent_i1_non_phi void (float, ptr): illegal VGPR to SGPR copy

@nickleus27
Copy link
Contributor

Thank you @chenzheng1030 ! I have been really busy lately, but when I have a chance I would like to circle back to this issue if no one else has picked it up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants