This is an archive of the discontinued LLVM Phabricator instance.

[FPEnv][EarlyCSE] Add support for CSE of constrained FP intrinsics
AbandonedPublic

Authored by kpn on Jul 13 2021, 8:15 AM.

Details

Summary

Currently we only CSE constrained FP intrinsics in the default floating point environment.

This patch adds support for CSE when exception behavior is "maytrap". No CSE of constrained intrinsics will happen across a function call or other unsafe event. Since any function call can change the floating point status register, the restrictions on exception behavior are required.

Diff Detail

Event Timeline

kpn created this revision.Jul 13 2021, 8:15 AM
kpn requested review of this revision.Jul 13 2021, 8:15 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 13 2021, 8:15 AM
kpn added a comment.Jul 13 2021, 8:25 AM

This patch may be overkill. If the reading of the floating point status register is restricted to "fpexcept.strict" then perhaps we don't need all of this code. But if we want to allow checking the status register after "fpexcept.maytrap" instructions then we do need all this. The LangRef restricts those reads to "fpexcept.strict". Are we certain we want to stick with that language?

This patch may be overkill. If the reading of the floating point status register is restricted to "fpexcept.strict" then perhaps we don't need all of this code. But if we want to allow checking the status register after "fpexcept.maytrap" instructions then we do need all this. The LangRef restricts those reads to "fpexcept.strict". Are we certain we want to stick with that language?

Where does the LangRef say this?

I think reading the status register should be allowed with fpexcept.maytrap, but the results of such a test must be understood to apply only to the generated code, not the strict semantics of the source. That is, if I check the status register and see, for example, that the DIVZERO flag is set, I can be sure that something in my code caused that, but if I check the register and no exception flags are set, that might have been because code that would have caused an exception was optimized away. The "maytrap" state is intended to avoid the introduction of spurious exceptions, but it isn't guaranteed to preserve all existing exceptions.

Strictly speaking, this is an off-the-map option. The relevant standards don't let you test the FP status flag or unmask exceptions unless fenv access is enabled, which leads to the fpexcept.strict behavior. The "maytrap" mode is meant to enable some useful optimizations, like DCE of code that might contain FP exceptions, while preventing optimizations that could cause rogue FP exceptions through speculative execution.

kpn planned changes to this revision.Oct 15 2021, 11:30 AM

OK, this is overkill per Andy's comment. I'll update.

kpn abandoned this revision.Oct 21 2021, 11:53 AM

Replaced with D112256. This patch here has restrictions that are not needed. To avoid confusing reading I'm replacing the review.