Skip to content

Conversation

@xyz-harshal
Copy link

@xyz-harshal xyz-harshal commented Nov 1, 2025

Fixes #164161

  • Define addlike PatFrags to match both ADD and disjoint OR
  • Update avx512_binop_rm multiclass chain to use SDPatternOperator
  • Apply addlike to VPADD instructions

Status: 34 test failures - need guidance on pattern scope

Issue: addlike Pattern Too Broad

The addlike pattern successfully matches disjoint OR operations, but it's being
applied too broadly and changing codegen in rotation/funnel-shift and other implementations.

- Define addlike PatFrags to match both ADD and disjoint OR
- Update avx512_binop_rm multiclass chain to use SDPatternOperator
- Apply addlike to VPADD instructions

Status: 34 test failures - need guidance on pattern scope
@github-actions
Copy link

github-actions bot commented Nov 1, 2025

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

@llvmbot llvmbot added backend:X86 llvm:SelectionDAG SelectionDAGISel as well labels Nov 1, 2025
@llvmbot
Copy link
Member

llvmbot commented Nov 1, 2025

@llvm/pr-subscribers-llvm-selectiondag

@llvm/pr-subscribers-backend-x86

Author: SYNC (xyz-harshal)

Changes

This PR is regarding issue #164161

  • Define addlike PatFrags to match both ADD and disjoint OR
  • Update avx512_binop_rm multiclass chain to use SDPatternOperator
  • Apply addlike to VPADD instructions

Status: 34 test failures - need guidance on pattern scope

Issue: addlike Pattern Too Broad

The addlike pattern successfully matches disjoint OR operations, but it's being
applied too broadly and changing codegen in rotation/funnel-shift and other implementations.

CC: @RKSimon


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

2 Files Affected:

  • (modified) llvm/include/llvm/Target/TargetSelectionDAG.td (+7)
  • (modified) llvm/lib/Target/X86/X86InstrAVX512.td (+14-14)
diff --git a/llvm/include/llvm/Target/TargetSelectionDAG.td b/llvm/include/llvm/Target/TargetSelectionDAG.td
index 07a858fd682fc..89e01885b62d6 100644
--- a/llvm/include/llvm/Target/TargetSelectionDAG.td
+++ b/llvm/include/llvm/Target/TargetSelectionDAG.td
@@ -1177,6 +1177,13 @@ def or_disjoint : PatFrag<(ops node:$lhs, node:$rhs),
   }];
 }
 
+def addlike : PatFrags<(ops node:$lhs, node:$rhs),
+                       [(add node:$lhs, node:$rhs), (or node:$lhs, node:$rhs)],[{
+  if (Op.getOpcode() == ISD::ADD)
+    return true;
+  return CurDAG->isADDLike(Op);
+}]>;
+
 def xor_like : PatFrags<(ops node:$lhs, node:$rhs),
                         [(xor node:$lhs, node:$rhs),
                          (or_disjoint node:$lhs, node:$rhs)]>;
diff --git a/llvm/lib/Target/X86/X86InstrAVX512.td b/llvm/lib/Target/X86/X86InstrAVX512.td
index 1b748b7355716..6ba01ea39f6f9 100644
--- a/llvm/lib/Target/X86/X86InstrAVX512.td
+++ b/llvm/lib/Target/X86/X86InstrAVX512.td
@@ -4685,7 +4685,7 @@ let Predicates = [HasVLX], AddedComplexity = 400 in {
 //===----------------------------------------------------------------------===//
 // AVX-512 - Integer arithmetic
 //
-multiclass avx512_binop_rm<bits<8> opc, string OpcodeStr, SDNode OpNode,
+multiclass avx512_binop_rm<bits<8> opc, string OpcodeStr, SDPatternOperator OpNode,
                            X86VectorVTInfo _, X86FoldableSchedWrite sched,
                            bit IsCommutable = 0> {
   defm rr : AVX512_maskable<opc, MRMSrcReg, _, (outs _.RC:$dst),
@@ -4704,7 +4704,7 @@ multiclass avx512_binop_rm<bits<8> opc, string OpcodeStr, SDNode OpNode,
                   Sched<[sched.Folded, sched.ReadAfterFold]>;
 }
 
-multiclass avx512_binop_rmb<bits<8> opc, string OpcodeStr, SDNode OpNode,
+multiclass avx512_binop_rmb<bits<8> opc, string OpcodeStr, SDPatternOperator OpNode,
                             X86VectorVTInfo _, X86FoldableSchedWrite sched,
                             bit IsCommutable = 0> :
            avx512_binop_rm<opc, OpcodeStr, OpNode, _, sched, IsCommutable> {
@@ -4719,7 +4719,7 @@ multiclass avx512_binop_rmb<bits<8> opc, string OpcodeStr, SDNode OpNode,
                   Sched<[sched.Folded, sched.ReadAfterFold]>;
 }
 
-multiclass avx512_binop_rm_vl<bits<8> opc, string OpcodeStr, SDNode OpNode,
+multiclass avx512_binop_rm_vl<bits<8> opc, string OpcodeStr, SDPatternOperator OpNode,
                               AVX512VLVectorVTInfo VTInfo,
                               X86SchedWriteWidths sched, Predicate prd,
                               bit IsCommutable = 0> {
@@ -4735,7 +4735,7 @@ multiclass avx512_binop_rm_vl<bits<8> opc, string OpcodeStr, SDNode OpNode,
   }
 }
 
-multiclass avx512_binop_rmb_vl<bits<8> opc, string OpcodeStr, SDNode OpNode,
+multiclass avx512_binop_rmb_vl<bits<8> opc, string OpcodeStr, SDPatternOperator OpNode,
                                AVX512VLVectorVTInfo VTInfo,
                                X86SchedWriteWidths sched, Predicate prd,
                                bit IsCommutable = 0> {
@@ -4751,7 +4751,7 @@ multiclass avx512_binop_rmb_vl<bits<8> opc, string OpcodeStr, SDNode OpNode,
   }
 }
 
-multiclass avx512_binop_rm_vl_q<bits<8> opc, string OpcodeStr, SDNode OpNode,
+multiclass avx512_binop_rm_vl_q<bits<8> opc, string OpcodeStr, SDPatternOperator OpNode,
                                 X86SchedWriteWidths sched, Predicate prd,
                                 bit IsCommutable = 0> {
   defm NAME : avx512_binop_rmb_vl<opc, OpcodeStr, OpNode, avx512vl_i64_info,
@@ -4759,14 +4759,14 @@ multiclass avx512_binop_rm_vl_q<bits<8> opc, string OpcodeStr, SDNode OpNode,
                                   REX_W, EVEX_CD8<64, CD8VF>;
 }
 
-multiclass avx512_binop_rm_vl_d<bits<8> opc, string OpcodeStr, SDNode OpNode,
+multiclass avx512_binop_rm_vl_d<bits<8> opc, string OpcodeStr, SDPatternOperator OpNode,
                                 X86SchedWriteWidths sched, Predicate prd,
                                 bit IsCommutable = 0> {
   defm NAME : avx512_binop_rmb_vl<opc, OpcodeStr, OpNode, avx512vl_i32_info,
                                   sched, prd, IsCommutable>, EVEX_CD8<32, CD8VF>;
 }
 
-multiclass avx512_binop_rm_vl_w<bits<8> opc, string OpcodeStr, SDNode OpNode,
+multiclass avx512_binop_rm_vl_w<bits<8> opc, string OpcodeStr, SDPatternOperator OpNode,
                                 X86SchedWriteWidths sched, Predicate prd,
                                 bit IsCommutable = 0> {
   defm NAME : avx512_binop_rm_vl<opc, OpcodeStr, OpNode, avx512vl_i16_info,
@@ -4774,7 +4774,7 @@ multiclass avx512_binop_rm_vl_w<bits<8> opc, string OpcodeStr, SDNode OpNode,
                                  WIG;
 }
 
-multiclass avx512_binop_rm_vl_b<bits<8> opc, string OpcodeStr, SDNode OpNode,
+multiclass avx512_binop_rm_vl_b<bits<8> opc, string OpcodeStr, SDPatternOperator OpNode,
                                 X86SchedWriteWidths sched, Predicate prd,
                                 bit IsCommutable = 0> {
   defm NAME : avx512_binop_rm_vl<opc, OpcodeStr, OpNode, avx512vl_i8_info,
@@ -4783,7 +4783,7 @@ multiclass avx512_binop_rm_vl_b<bits<8> opc, string OpcodeStr, SDNode OpNode,
 }
 
 multiclass avx512_binop_rm_vl_dq<bits<8> opc_d, bits<8> opc_q, string OpcodeStr,
-                                 SDNode OpNode, X86SchedWriteWidths sched,
+                                 SDPatternOperator OpNode, X86SchedWriteWidths sched,
                                  Predicate prd, bit IsCommutable = 0> {
   defm Q : avx512_binop_rm_vl_q<opc_q, OpcodeStr#"q", OpNode, sched, prd,
                                    IsCommutable>;
@@ -4793,7 +4793,7 @@ multiclass avx512_binop_rm_vl_dq<bits<8> opc_d, bits<8> opc_q, string OpcodeStr,
 }
 
 multiclass avx512_binop_rm_vl_bw<bits<8> opc_b, bits<8> opc_w, string OpcodeStr,
-                                 SDNode OpNode, X86SchedWriteWidths sched,
+                                 SDPatternOperator OpNode, X86SchedWriteWidths sched,
                                  Predicate prd, bit IsCommutable = 0> {
   defm W : avx512_binop_rm_vl_w<opc_w, OpcodeStr#"w", OpNode, sched, prd,
                                    IsCommutable>;
@@ -4804,7 +4804,7 @@ multiclass avx512_binop_rm_vl_bw<bits<8> opc_b, bits<8> opc_w, string OpcodeStr,
 
 multiclass avx512_binop_rm_vl_all<bits<8> opc_b, bits<8> opc_w,
                                   bits<8> opc_d, bits<8> opc_q,
-                                  string OpcodeStr, SDNode OpNode,
+                                  string OpcodeStr, SDPatternOperator OpNode,
                                   X86SchedWriteWidths sched,
                                   bit IsCommutable = 0> {
   defm NAME : avx512_binop_rm_vl_dq<opc_d, opc_q, OpcodeStr, OpNode,
@@ -4815,7 +4815,7 @@ multiclass avx512_binop_rm_vl_all<bits<8> opc_b, bits<8> opc_w,
 
 multiclass avx512_binop_rm2<bits<8> opc, string OpcodeStr,
                             X86FoldableSchedWrite sched,
-                            SDNode OpNode,X86VectorVTInfo _Src,
+                            SDPatternOperator OpNode,X86VectorVTInfo _Src,
                             X86VectorVTInfo _Dst, X86VectorVTInfo _Brdct,
                             bit IsCommutable = 0> {
   defm rr : AVX512_maskable<opc, MRMSrcReg, _Dst, (outs _Dst.RC:$dst),
@@ -4847,7 +4847,7 @@ multiclass avx512_binop_rm2<bits<8> opc, string OpcodeStr,
   }
 }
 
-defm VPADD : avx512_binop_rm_vl_all<0xFC, 0xFD, 0xFE, 0xD4, "vpadd", add,
+defm VPADD : avx512_binop_rm_vl_all<0xFC, 0xFD, 0xFE, 0xD4, "vpadd", addlike,
                                     SchedWriteVecALU, 1>;
 defm VPSUB : avx512_binop_rm_vl_all<0xF8, 0xF9, 0xFA, 0xFB, "vpsub", sub,
                                     SchedWriteVecALU, 0>;
@@ -4882,7 +4882,7 @@ multiclass avx512_binop_all<bits<8> opc, string OpcodeStr,
                             X86SchedWriteWidths sched,
                             AVX512VLVectorVTInfo _SrcVTInfo,
                             AVX512VLVectorVTInfo _DstVTInfo,
-                            SDNode OpNode, list<Predicate> prds512,
+                            SDPatternOperator OpNode, list<Predicate> prds512,
                             list<Predicate> prds,
                             X86VectorVTInfo _VTInfo512 = _SrcVTInfo.info512,
                             X86VectorVTInfo _VTInfo256 = _SrcVTInfo.info256,

Copy link
Collaborator

@RKSimon RKSimon left a comment

Choose a reason for hiding this comment

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

Please can you regenerate the failing tests with the update_llc_test_checks.py script

@xyz-harshal
Copy link
Author

Please can you regenerate the failing tests with the update_llc_test_checks.py script

Done 👍🏽

@xyz-harshal
Copy link
Author

@RKSimon
7 tests remain failing but are out of scope for this SelectionDAG optimization.
what can be done here?
All SelectionDAG-based tests have been regenerated and now pass with the new codegen

@RKSimon RKSimon self-requested a review November 18, 2025 19:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backend:X86 llvm:SelectionDAG SelectionDAGISel as well

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[X86] Failure to fold or disjoint nodes into a AVX512 predicated add instruction

3 participants