[llvm] [SDAG] Make Select-with-Identity-Fold More Flexible; NFC (PR #136554)

[Marius Kamp via llvm-commits]

================
@@ -3351,13 +3351,26 @@ class TargetLoweringBase {
   }
 
   /// Return true if pulling a binary operation into a select with an identity
-  /// constant is profitable. This is the inverse of an IR transform.
+  /// constant is profitable for the given binary operation and type. This is
+  /// the inverse of an IR transform.
   /// Example: X + (Cond ? Y : 0) --> Cond ? (X + Y) : X
   virtual bool shouldFoldSelectWithIdentityConstant(unsigned BinOpcode,
                                                     EVT VT) const {
     return false;
   }
 
+  /// Return true if pulling a binary operation into a select with an identity
+  /// constant is profitable for the given binary operation, select operation,
+  /// operand to the binary operation, and select operand that is not the
+  /// identity constant. This is a more fine-grained variant of the previous
+  /// overload that is called only if the previous overload returned true.
+  virtual bool
+  shouldFoldSelectWithIdentityConstant(unsigned BinOpcode,
+                                       unsigned SelectOpcode, SDValue X,
+                                       SDValue NonIdConstNode) const {
+    return SelectOpcode == ISD::VSELECT;
+  }
----------------
mskamp wrote:

But the call to `shouldFoldSelectWithIdentityConstant()` in `foldSelectWithIdentityConstant()` depends on the true/false operand of the select, which is only available after handling commutativity of the binary operator. Therefore, we cannot sink the handling of commutativity beyond the call to `shouldFoldSelectWithIdentityConstant()`.

Nevertheless, I've noticed that the mentioned worst case with four calls to `shouldFoldSelectWithIdentityConstant()` can only happen if we have a select with an identity constant as both the true and false operand. But such operations should have been folded before, so we should have at most two calls to `shouldFoldSelectWithIdentityConstant()`. Therefore, just merging the methods and keeping only the second call is probably not an issue for performance. I've implemented this suggestion.

https://github.com/llvm/llvm-project/pull/136554