[LoopInterchange] Reject cases where ninf flag is present

[madhur13490]

When this flag is present on instructions, semantics guarantee that input/output will not +/- inf. Interchange may lead to the result being +/- inf. I think when this flag is present on the instructions, we should reject interchange and bail out.

LLVM fast-math flags: https://llvm.org/docs/LangRef.html#fast-math-flags

This can be considered as an extension to #148612

[madhur13490]

@kasuga-fj @sjoerdmeijer Does this make sense? If so, I would like to take a look.

[kasuga-fj]

As I understand it, interchanging is rejected if there is a floating-point instruction for which the reassoc flag it not set.

llvm-project/llvm/lib/Transforms/Scalar/LoopInterchange.cpp

Lines 824 to 825 in c2e41be

	if (RD.getExactFPMathInst() != nullptr)
	return nullptr;

Do you have any example where this check is insufficient?

[madhur13490]

https://godbolt.org/z/93ddenbM4
This is one of the tests you had added in the mentioned patch. fadd has both ninf and reassoc set. reassoc implies loop interchange can be performed, but ninf should disable it because interchange may lead to overflow. I am not sure if reassoc has more precedence.

[kasuga-fj]

I didn't add such case in that patch, but it is just an oversight, and it looks to me that we should also take it into account.

As reading the section about Rewrite-based flags, the handling of ninf flag appears to be independent of reassoc. It states that:

These rules do not apply to the other fast-math flags. Whether or not a flag like nnan is present on any or all of the rewritten instructions is based on whether or not it is possible for said instruction to have a NaN input or output, given the original flags.

IIUIC, reassoc allows transformations based on associativity in mathematical sense, even if they can change the floating-point result. Therefore, remaining the ninf flag in your example without any validation seems to be incorrect (and maybe we can't make any guarantees about the resulting values). Probably the same applies to other fast-math flags (fast, nnan and nsz). However, I'm not sure which is better in this case: applying the interchange and removing the ninf, or avoiding it to preserve the ninf.

I also realized that it would also be necessary to replace the rewrite-based flags. Consider a variant of your example. Here is a part of the loop body:

...
%prod.j.next.0 = fmul reassoc float %prod.j, %a
%prod.j.next = fmul reassoc arcp float %prod.j.next.0, %f
...

Considering the example in the document, probably the arcp flag should be dropped from the latter instruction after the interchange.

Here are suggestions to these issues:

• Don't perform interchanges if there are any flags other than reassoc exist
• Drop them after interchanges
• Only allow interchanges when the reduction involves a single instruction
  • In this approach, we don't need to consider about other rewrite-based flags, I think

Note: I believe the problems would arise only with either FAdd or FMul reductions.

[madhur13490]

I didn't add such case in that patch, but it is just an oversight, and it looks to me that we should also take it into account.

Here is the PR to add the test: #149090

Here are suggestions to these issues:

• Don't perform interchanges if there are any flags other than reassoc that exist

I agree with this. I think it is safe to do so. I can propose a patch later. @sjoerdmeijer any thoughts?

• Drop them after interchanges

Why drop if we don't perform interchange?

• Only allow interchanges when the reduction involves a single instruction

Do you mean a single reduction happening in the loop?

[kasuga-fj]

• Drop them after interchanges

Why drop if we don't perform interchange?

I intended to mention three distinct ways to address this issue. Dropping the flags would be another approach, instead of rejecting interchanges, similar to removing nuw/nsw flags after interchanges, as in #148612. However, fast-math flags seem to have a greater impact on optimizations than nuw/nsw flags, so I'm not sure if this is a good idea.

• Only allow interchanges when the reduction involves a single instruction

Do you mean a single reduction happening in the loop?

No, I meant a case where all reductions consist of a single floating-point instruction (e.g., a tightly nested loops whose body is sum += a[i][j]; prod *= a[i][j]). This is also another approach I came up with. If a reduction consists of multiple instructions, as in the example in my previous comment (where a single reduction has multiple instructions like prod *= a[i][j]; prod *= b[i][j];), we probably need to check the rewrite-based flags as well. It may be trickier to handle this case correctly, but if the reduction has only a single instruction, as I understand it, it should be safe to interchange the loops without checking those flags. However, fast-math flags still need to be handled (maybe removed) properly.