[DAGCombiner] Remove hasOneUse check from sext+sext_inreg to sext_inreg combine #140207
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This stack of pull requests is managed by Graphite. Learn more about stacking. |
This was referenced May 16, 2025
|
@llvm/pr-subscribers-llvm-selectiondag Author: Pierre van Houtryve (Pierre-vh) ChangesThe hasOneUseCheck does not really add anything and makes the combine too Full diff: https://github.com/llvm/llvm-project/pull/140207.diff 1 Files Affected:
diff --git a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
index d6e288a59b2ee..79949bb6d37a0 100644
--- a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
@@ -14101,7 +14101,7 @@ SDValue DAGCombiner::visitSIGN_EXTEND(SDNode *N) {
}
// If the trunc wasn't legal, try to fold to (sext_inreg (anyext x))
- if ((!LegalTypes || TLI.isTypeLegal(VT)) && N0.hasOneUse()) {
+ if (!LegalTypes || TLI.isTypeLegal(VT)) {
SDValue ExtSrc = DAG.getAnyExtOrTrunc(N00, DL, VT);
return DAG.getNode(ISD::SIGN_EXTEND_INREG, DL, VT, ExtSrc,
N0->getOperand(1));
|
Pierre-vh
commented
May 16, 2025
| @@ -14101,7 +14101,7 @@ SDValue DAGCombiner::visitSIGN_EXTEND(SDNode *N) { | ||
| } | ||
|
|
||
| // If the trunc wasn't legal, try to fold to (sext_inreg (anyext x)) | ||
| if ((!LegalTypes || TLI.isTypeLegal(VT)) && N0.hasOneUse()) { | ||
| if (!LegalTypes || TLI.isTypeLegal(VT)) { |
There was a problem hiding this comment.
There are no test changes, but in the next PR some tests would regress without it, in particular s_test_smed3_i8_pat_0 would have a mix of s_sext_i32_i8 and s_bfe + s_sext_i32_i16 chained together.
arsenm
approved these changes
May 16, 2025
Merge activity
|
9ed6224 to
c0fbfb3
Compare
aa2e1c7 to
cd12b2c
Compare
…eg combine The hasOneUseCheck does not really add anything and makes the combine too restrictive. Upcoming patches benefit from removing the hasOneUse check.
cd12b2c to
bb86037
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The hasOneUseCheck does not really add anything and makes the combine too
restrictive. Upcoming patches benefit from removing the hasOneUse check.