[llvm] [InstCombine] Fold copysign(floor(fabs(X)), X) to trunc(X) (PR #200836)

[Aayush Shrivastava via llvm-commits]

================
@@ -3119,8 +3119,11 @@ Instruction *InstCombinerImpl::visitCallInst(CallInst &CI) {
     // copysign(floor(fabs(X)), X) --> copysign(trunc(X), X)
     // copysign ignores the sign bit of its magnitude argument (implicit fabs),
     // so replacing floor(fabs(X)) with trunc(X) is correct for all inputs
-    // including NaN without requiring nnan.
-    if (match(Mag, m_Intrinsic<Intrinsic::floor>(m_FAbs(m_Specific(Sign))))) {
+    // including NaN without requiring nnan. Use stripSignOnlyFPOps inside the
+    // fabs to also handle fabs(fneg(X)) and fabs(copysign(X,Y)) patterns.
+    Value *FAbsArg;
+    if (match(Mag, m_Intrinsic<Intrinsic::floor>(m_FAbs(m_Value(FAbsArg)))) &&
+        stripSignOnlyFPOps(FAbsArg) == Sign) {
----------------
iamaayushrivastava wrote:

Thanks for the suggestion! After testing, removing `m_FAbs` entirely causes incorrect folds. For example, `copysign(floor(fneg(fabs(X))), X)` would incorrectly fold to `copysign(trunc(X), X)` since `floor(-|X|) ≠ floor(|X|)` for non-integer X (e.g. X=2.7 gives 3.0 vs 2.0). The `m_FAbs` is needed to guarantee the floor argument is always non-negative. WriI've updated the comment to clarify this. Happy to discuss further if I'm missing something.

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