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

[llvm][NVPTX] Fix RAUW bug in NVPTXProxyRegErasure #105871

Merged
merged 4 commits into from
Aug 24, 2024
Merged

Conversation

Mogball
Copy link
Contributor

@Mogball Mogball commented Aug 23, 2024

Fix bug introduced in #105730

The bug is in how the batch RAUW is implemented. If we have

%0 = mov %src
%1 = mov %0

use %0
use %1

The use of %1 is rewritten to %0, not %src. This PR just looks for a replacement when it maps to the src register, which should transitively propagate the replacements.

@llvmbot
Copy link
Member

llvmbot commented Aug 23, 2024

@llvm/pr-subscribers-backend-nvptx

Author: Jeff Niu (Mogball)

Changes

Fix bug introduced in #105730


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

2 Files Affected:

  • (modified) llvm/lib/Target/NVPTX/NVPTXProxyRegErasure.cpp (+5-1)
  • (added) llvm/test/CodeGen/NVPTX/bug105730.ll (+17)
diff --git a/llvm/lib/Target/NVPTX/NVPTXProxyRegErasure.cpp b/llvm/lib/Target/NVPTX/NVPTXProxyRegErasure.cpp
index f3a3362addb0ea..16c2b307efabfb 100644
--- a/llvm/lib/Target/NVPTX/NVPTXProxyRegErasure.cpp
+++ b/llvm/lib/Target/NVPTX/NVPTXProxyRegErasure.cpp
@@ -78,7 +78,11 @@ bool NVPTXProxyRegErasure::runOnMachineFunction(MachineFunction &MF) {
         assert(InOp.isReg() && "ProxyReg input should be a register.");
         assert(OutOp.isReg() && "ProxyReg output should be a register.");
         RemoveList.push_back(&MI);
-        RAUWBatch.try_emplace(OutOp.getReg(), InOp.getReg());
+        Register replacement = InOp.getReg();
+        // Check if the replacement itself has been replaced.
+        if (auto it = RAUWBatch.find(replacement); it != RAUWBatch.end())
+          replacement = it->second;
+        RAUWBatch.try_emplace(OutOp.getReg(), replacement);
         break;
       }
       }
diff --git a/llvm/test/CodeGen/NVPTX/bug105730.ll b/llvm/test/CodeGen/NVPTX/bug105730.ll
new file mode 100644
index 00000000000000..718e7ca6b80fd8
--- /dev/null
+++ b/llvm/test/CodeGen/NVPTX/bug105730.ll
@@ -0,0 +1,17 @@
+; RUN: llc < %s -verify-machineinstrs
+
+; Check that llc doesn't crash.
+
+target triple = "nvptx64-nvidia-cuda"
+
+define void @__builtin_splat_i8(i32 %0) {
+.lr.ph:
+  %1 = trunc i32 %0 to i8
+  %broadcast.splatinsert = insertelement <4 x i8> poison, i8 %1, i64 0
+  %broadcast.splat = shufflevector <4 x i8> %broadcast.splatinsert, <4 x i8> poison, <4 x i32> zeroinitializer
+  br label %vector.body
+
+vector.body:
+  store <4 x i8> %broadcast.splat, ptr addrspace(1) poison, align 1
+  br label %vector.body
+}

@Mogball
Copy link
Contributor Author

Mogball commented Aug 24, 2024

I'm getting more reports of breakages coming from downstream projects, so I'm going to go ahead and land this to fix-forward. If you have any more comments on the PR, I am happy to address them in follow ups! Thanks for the review and testing tips

@Mogball Mogball merged commit 31b4bf9 into main Aug 24, 2024
6 of 8 checks passed
@Mogball Mogball deleted the users/mogball/bug branch August 24, 2024 17:19
ThomasRaoux pushed a commit to triton-lang/triton that referenced this pull request Sep 1, 2024
This bumps to llvm/llvm-project@56152fa377 to further
include the following fix
* llvm/llvm-project#105871

Which was fixing issues in a previous LLVM bump
* #4624
dmpolukhin pushed a commit to dmpolukhin/llvm-project that referenced this pull request Sep 2, 2024
Fix bug introduced in llvm#105730

The bug is in how the batch RAUW is implemented. If we have 

```
%0 = mov %src
%1 = mov %0

use %0
use %1
```

The use of `%1` is rewritten to `%0`, not `%src`. This PR just looks for
a replacement when it maps to the src register, which should
transitively propagate the replacements.
bertmaher pushed a commit to bertmaher/triton that referenced this pull request Dec 10, 2024
This bumps to llvm/llvm-project@56152fa377 to further
include the following fix
* llvm/llvm-project#105871

Which was fixing issues in a previous LLVM bump
* triton-lang#4624
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.

4 participants