This is an archive of the discontinued LLVM Phabricator instance.

Pass for fixing dependencies of constrained intrinsics
AbandonedPublic

Authored by sepavloff on Jan 26 2022, 11:01 PM.

Details

Summary

Constrained intrinsics model the interaction with floating point
environment as side effect. It creates problems if some transformation
replaces an intrinsic call and leaves the call dangling. As the call has
side effect, it cannot be removed by DCE. On the other hand, in many
cases the side effect is absent or may be ignored and it is safe to
remove such call.

In particular, such problem exists in constant folding. Intrinsic call
cannot be modified inside constant evaluator so it must be made somewhere
in the transformation pass. There are several passes that could call
constant evaluator, putting cleanup code to each of them does not look
like a flexible solution. Instead, this patch introduces a pass that makes
the cleanup in one place. The pass is executed late in a pipeline, when
all relevant replacements have already done.

Although this change is made for solving constant evaluation problems,
removing floating-point operation side effect is useful in other cases
also. For example, if FP exceptions are ignored but rounding mode is
non-default, side effect may be removed in many cases, it can produce
faster code. The new pass could be used for this task also, but in this
case it must be executed also somewhere earlier in pipeline.

Diff Detail

Event Timeline

sepavloff created this revision.Jan 26 2022, 11:01 PM
sepavloff requested review of this revision.Jan 26 2022, 11:01 PM
Herald added a project: Restricted Project. · View Herald TranscriptJan 26 2022, 11:01 PM
nikic added a comment.Jan 27 2022, 2:27 AM

I think it would be better to do this as part of InstCombine. I don't think having a single pass at the end of the pipeline would be a good solution to this, as it means that these dead calls will survive through most of the optimization pipeline, blocking cost models and other optimizations. This doesn't look expensive enough that including it in InstCombine would be a problem.

kpn added a comment.Jan 27 2022, 9:27 AM

I think it would be better to do this as part of InstCombine. I don't think having a single pass at the end of the pipeline would be a good solution to this, as it means that these dead calls will survive through most of the optimization pipeline, blocking cost models and other optimizations. This doesn't look expensive enough that including it in InstCombine would be a problem.

True, we don't want instructions sticking around nearly as long as this patch. I submit, though, that we should be removing instructions in the transform passes that have been taught how to remove them. I don't see the value keeping dead instructions around after a transform pass should have removed them.

And if an instruction will _never_ be executed then I don't see the harm in DCE removing it. We wouldn't be removing a potential trap from runtime because the code won't be executed and the DCE pass knows it.

llvm/test/Transforms/FloatDependencyFixup/float-dep-fixup.ll
21

Why is this not getting removed as well?

sepavloff abandoned this revision.Jan 27 2022, 7:58 PM

I think it would be better to do this as part of InstCombine. I don't think having a single pass at the end of the pipeline would be a good solution to this, as it means that these dead calls will survive through most of the optimization pipeline, blocking cost models and other optimizations. This doesn't look expensive enough that including it in InstCombine would be a problem.

Indeed, this particular task can be solved in InstCombiner. It wouldn't allow to remove side effect in more complex cases, but this patch does not provide a solution for such. The fix in InstCombiner is implemented in D118426.

And if an instruction will _never_ be executed then I don't see the harm in DCE removing it. We wouldn't be removing a potential trap from runtime because the code won't be executed and the DCE pass knows it.

The problem is that the instruction is executed but its result is not used, so it can be safely removed.

llvm/test/Transforms/FloatDependencyFixup/float-dep-fixup.ll
21

This function has a side effect: it raises inexact exception. So removing it produces function that is not equivalent to the original.