Skip to content

[AMDGPU] Add sext_trunc in RegBankCombiner #131623

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

Merged
merged 3 commits into from
Apr 14, 2025

Conversation

Pierre-vh
Copy link
Contributor

No description provided.

Copy link
Contributor Author

Pierre-vh commented Mar 17, 2025

@Pierre-vh Pierre-vh marked this pull request as ready for review March 17, 2025 14:44
@llvmbot
Copy link
Member

llvmbot commented Mar 17, 2025

@llvm/pr-subscribers-llvm-globalisel

@llvm/pr-subscribers-backend-amdgpu

Author: Pierre van Houtryve (Pierre-vh)

Changes

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

1 Files Affected:

  • (modified) llvm/lib/Target/AMDGPU/AMDGPUCombine.td (+1-1)
diff --git a/llvm/lib/Target/AMDGPU/AMDGPUCombine.td b/llvm/lib/Target/AMDGPU/AMDGPUCombine.td
index a21505356274b..083ce48911689 100644
--- a/llvm/lib/Target/AMDGPU/AMDGPUCombine.td
+++ b/llvm/lib/Target/AMDGPU/AMDGPUCombine.td
@@ -181,5 +181,5 @@ def AMDGPURegBankCombiner : GICombiner<
    zext_trunc_fold, int_minmax_to_med3, ptr_add_immed_chain,
    fp_minmax_to_clamp, fp_minmax_to_med3, fmed3_intrinsic_to_clamp,
    identity_combines, redundant_and, constant_fold_cast_op,
-   cast_of_cast_combines]> {
+   cast_of_cast_combines, sext_trunc]> {
 }

Copy link
Contributor

@arsenm arsenm left a comment

Choose a reason for hiding this comment

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

Missing test changes?

@Pierre-vh Pierre-vh force-pushed the users/pierre-vh/sext_trunc_in_amddgpu_rbcomb branch from 3f2cbbd to 4feac2f Compare March 18, 2025 07:53
@Pierre-vh Pierre-vh requested a review from arsenm March 18, 2025 07:55
@Pierre-vh
Copy link
Contributor Author

Test changes were in the previous diff in the stack, it should be fixed now.

@shiltian
Copy link
Contributor

Hmm, I still can't see the test change?

@Pierre-vh
Copy link
Contributor Author

Ah, this doesn't do anything at this stage. It's only helpful once we disable widening of i16 ops to i32 in CGP. Then this pattern can appear and it'll fold it.

This combine is tested in AArch64. Should I copy over a few simple test cases in the AMDGPU folder just to show the combine works in RegBankCombiner?

Base automatically changed from users/pierre-vh/sext-trunc-to-sext-inreg to main March 24, 2025 08:32
@Pierre-vh Pierre-vh force-pushed the users/pierre-vh/sext_trunc_in_amddgpu_rbcomb branch 2 times, most recently from f95a7c0 to e717745 Compare March 24, 2025 11:14
@@ -1,5 +1,6 @@
# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py
# RUN: llc -mtriple=amdgcn-mesa-mesa3d -mcpu=tahiti -run-pass=amdgpu-prelegalizer-combiner -verify-machineinstrs %s -o - | FileCheck -check-prefix=GCN %s
# RUN: llc -mtriple=amdgcn-mesa-mesa3d -mcpu=tahiti -run-pass=amdgpu-prelegalizer-combiner -verify-machineinstrs %s -o - | FileCheck -check-prefixes=GCN,PRELEGAL %s
# RUN: llc -mtriple=amdgcn-mesa-mesa3d -mcpu=tahiti -run-pass=amdgpu-regbank-combiner -verify-machineinstrs %s -o - | FileCheck -check-prefixes=GCN,RBCOMB %s
Copy link
Contributor

Choose a reason for hiding this comment

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

@arsenm Is it a good idea here to test two passes in one test case?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes. It helps to capture the lowering at significant points during the codegen that are important to test in certain scenarios.

@Pierre-vh Pierre-vh requested review from shiltian and cdevadas March 28, 2025 13:22
@Pierre-vh Pierre-vh requested a review from jmmartinez April 10, 2025 13:45
Copy link
Contributor Author

Pierre-vh commented Apr 14, 2025

Merge activity

  • Apr 14, 4:03 AM EDT: A user started a stack merge that includes this pull request via Graphite.
  • Apr 14, 4:06 AM EDT: Graphite rebased this pull request as part of a merge.
  • Apr 14, 4:10 AM EDT: Graphite rebased this pull request as part of a merge.
  • Apr 14, 4:13 AM EDT: Graphite rebased this pull request as part of a merge.
  • Apr 14, 4:15 AM EDT: A user merged this pull request with Graphite.

@Pierre-vh Pierre-vh force-pushed the users/pierre-vh/sext_trunc_in_amddgpu_rbcomb branch 2 times, most recently from dd4dbe6 to ca9b7c3 Compare April 14, 2025 08:09
@Pierre-vh Pierre-vh force-pushed the users/pierre-vh/sext_trunc_in_amddgpu_rbcomb branch from ca9b7c3 to 2f2f6b7 Compare April 14, 2025 08:12
@Pierre-vh Pierre-vh merged commit 931a78a into main Apr 14, 2025
6 of 11 checks passed
@Pierre-vh Pierre-vh deleted the users/pierre-vh/sext_trunc_in_amddgpu_rbcomb branch April 14, 2025 08:15
var-const pushed a commit to ldionne/llvm-project that referenced this pull request Apr 17, 2025
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.

5 participants