dtcxzyw wrote: > This patch is motivated by an internal benchmark where there were some cases > where this helped, though even that case is in some sense artificial.
It is ok to optimize some pattern in proxy apps/benchmarks :) > Is this a necessary criteria for landing this change? As recursively computing known bits/fpclass is expensive, it is not worth adding additional complexity unless it is useful for real-world applications. > I believe we already handle float to int in KnownBits I posted https://github.com/llvm/llvm-project/pull/86409 because I found it enabled more optimizations in [my ir dataset] (https://github.com/dtcxzyw/llvm-opt-benchmark/pull/451/files). > and adding the inverse in KnownFPClass seems like a correct and reasonable > extension of the logic, ***even if there are not many cases where it is > used.*** Don't do this. It is a trap of code coverage. We need to strike a balance between optimizations, compilation time and maintainability. For more details, see https://llvm.org/docs/InstCombineContributorGuide.html#real-world-usefulness. https://github.com/llvm/llvm-project/pull/97762 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
