This is an archive of the discontinued LLVM Phabricator instance.

[FPEnv][InstSimplify] Enable more folds for constrained fsub
Needs ReviewPublic

Authored by kpn on Aug 2 2021, 9:19 AM.

Details

Summary

Currently there are optimizations for the fsub instruction that do not fire for a constrained fsub. Add some of these optimizations.

Optimizations that require teaching the IR matchers about the constrained intrinsics are for a future patch.

Diff Detail

Event Timeline

kpn created this revision.Aug 2 2021, 9:19 AM
kpn requested review of this revision.Aug 2 2021, 9:19 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 2 2021, 9:19 AM
sepavloff added inline comments.Aug 2 2021, 11:03 AM
llvm/lib/Analysis/InstructionSimplify.cpp
5031–5033

This transformation is valid only if FMF.noNaNs() is set, because it is incorrect if X==SNaN. Similar for other cases below.

kpn updated this revision to Diff 375582.Sep 28 2021, 7:59 AM

Update for review comments. Instructions that are "maytrap" where a SNaN may be seen must not be eliminated.

I've also added tests for transforms that won't fire due to a lack of IR matcher support. We're going through codepaths we weren't before so we're testing what we weren't before.

Please add the test file as a preliminary commit. Then update here to show the code diffs. Also as part of this patch, apply comments on the tests that are intentionally not changing and/or are expected to change later (TODO).

llvm/lib/Analysis/InstructionSimplify.cpp
5031–5033

This clause is going to be repeated in many places. Make it or its inverse a helper function and give it a name (canIgnoreSNAN or mustPreserveSNAN?).

kpn updated this revision to Diff 379117.Oct 12 2021, 11:18 AM

Update for review comments.

lebedev.ri resigned from this revision.Jan 12 2023, 5:31 PM

This review may be stuck/dead, consider abandoning if no longer relevant.
Removing myself as reviewer in attempt to clean dashboard.

Herald added a project: Restricted Project. · View Herald TranscriptJan 12 2023, 5:31 PM
Herald added a subscriber: StephenFan. · View Herald Transcript