[SelectionDAG] Fold (avg x, 0) -> x >> 1 #85581
Conversation
|
@llvm/pr-subscribers-backend-aarch64 @llvm/pr-subscribers-llvm-selectiondag Author: AtariDreams (AtariDreams) ChangesFull diff: https://github.com/llvm/llvm-project/pull/85581.diff 1 Files Affected:
diff --git a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
index 5eb53d57c9c2bf..f0cd23a4e65b9d 100644
--- a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
@@ -5083,7 +5083,12 @@ SDValue DAGCombiner::visitAVG(SDNode *N) {
if (N0 == N1 && Level >= AfterLegalizeTypes)
return N0;
- // TODO If we use avg for scalars anywhere, we can add (avgfl x, 0) -> x >> 1
+ // Fold (avg x, 0) -> x >> 1
+ if (isNullOrNullSplat(N0))
+ return DAG.getNode(ISD::SRL, DL, VT, N1, DAG.getConstant(1, DL, VT));
+
+ if (isNullOrNullSplat(N1))
+ return DAG.getNode(ISD::SRL, DL, VT, N0, DAG.getConstant(1, DL, VT));
return SDValue();
}
|
| @@ -5083,7 +5083,18 @@ SDValue DAGCombiner::visitAVG(SDNode *N) { | |||
| if (N0 == N1 && Level >= AfterLegalizeTypes) | |||
| return N0; | |||
|
|
|||
| // TODO If we use avg for scalars anywhere, we can add (avgfl x, 0) -> x >> 1 | |||
| // Fold (avg x, 0) -> x >> 1 | |||
| if (isNullOrNullSplat(N0)) | |||
There was a problem hiding this comment.
Don't we canonicalize constants to RHS earlier?
There was a problem hiding this comment.
Can't you just remove the isVector check around the existing code on line 5067?
There was a problem hiding this comment.
Not the same behavior. isNullOrNullSplat returns false if there are any undefs in the vector.
There was a problem hiding this comment.
Then replace that code with your preferred version.
There was a problem hiding this comment.
Also if you are intentionally improving the handling of vectors with undefs then you should add tests for that.
There was a problem hiding this comment.
isConstantSplatVectorAllZeros is for vectors. isNullOrNullSplat is for constants and vectors.
There was a problem hiding this comment.
I am not at all changing how vectors with undefs are handled. I am only changing how zero constants/vectors are handled.
There was a problem hiding this comment.
If you pass AllowUndefs=true to isNullOrNullSplat, can we remove the ISD::isConstantSplatVectorAllZeros version of the code?
| @@ -464,8 +464,7 @@ define <8 x i16> @rhaddu_i_const_lhs(<8 x i16> %src1) { | |||
| define <8 x i16> @rhaddu_i_const_zero(<8 x i16> %src1) { | |||
| ; CHECK-LABEL: rhaddu_i_const_zero: | |||
| ; CHECK: // %bb.0: | |||
| ; CHECK-NEXT: movi v1.2d, #0000000000000000 | |||
| ; CHECK-NEXT: urhadd v0.8h, v0.8h, v1.8h | |||
| ; CHECK-NEXT: ushr v0.8h, v0.8h, #1 | |||
There was a problem hiding this comment.
I think these would be incorrect for avgceil.
AtariDreams
force-pushed
the
avg
branch
5 times, most recently
from
b3b6578
to
a99f4a3
Compare
|
I still don't understand why we need two implementations of "fold (avgfloor x, 0) -> x >> 1" in the same function, so it would be a nack from me. Maybe one of the other reviewers can understand it. |
|
✅ With the latest revision this PR passed the Python code formatter. |
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
| return DAG.getNode(ISD::SRA, DL, VT, N0, DAG.getConstant(1, DL, VT)); | ||
| if (Opcode == ISD::AVGFLOORU) | ||
| return DAG.getNode(ISD::SRL, DL, VT, N0, DAG.getConstant(1, DL, VT)); | ||
| } |
There was a problem hiding this comment.
Remove the vector folds above? (github won't let me put this comment there)
No description provided.