[PATCH] D101727: Fix PR47960 - Incorrect transformation of fabs with nnan flag

[Sanjay Patel via Phabricator via llvm-commits]

spatel added a comment.

The current code was added with rG8b6d9f6 <https://reviews.llvm.org/rG8b6d9f60ed712529d0f4f605387fe8045c96c1b1> and a reaction/partial fix for:
https://llvm.org/PR38086

I was afraid that relying on the `select` for FMF could hinder further optimizations because propagation of FMF to `select` was (and still is) incomplete.
But I don't have any clear evidence to show the effect either way, so I think it's fine to give this a try. Just be on the lookout for regressions.



================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp:2804-2805
   // sign of a NaN value.
   // FIXME: These folds should test/propagate FMF from the select, not the
   //        fsub or fneg.
   // (X <= +/-0.0) ? (0.0 - X) : X --> fabs(X)
----------------
Remove the FIXME code comment.


================
Comment at: llvm/test/Transforms/InstCombine/fabs.ll:268
 
 ; X <= 0.0 ? (0.0 - X) : X --> fabs(X)
 
----------------
Add a test comment here to make it explicit that `nnan` knowledge is lost with the transform.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D101727/new/

https://reviews.llvm.org/D101727