Questions about Floating-Point Operations in MLIR

1. Choice of Operations in atomic_rmw

I have a question regarding the choice of operations in this section of the code. Why are we not using arith.maximumf and arith.minimumf here instead of the cmpf + select combination? Is there a specific reason for this choice? It seems more consistent to use these specific operations, especially given the future introduction of arith.maxnumf and arith.minnumf operations, along with atomic_rmw.maxnumf and atomic_rmw.minnumf. If we keep the cmpf + select pattern, the code generation is going to be the same for pairs of min/max operations, which seems not quite correct.

2. Folding Patterns

I also noticed that there are relatively few folding patterns for arith.maximumf and arith.minimumf in the codebase. For instance, here, there are only three patterns implemented:

a) maxf(x,x) -> x
b) maxf(x, -inf) -> x
c) Constant folding

However, I can think of several additional patterns that could be useful:

d) maxf(-inf, x) -> x
e) maxf(+inf, x) -> +inf
g) maxf(x, +inf) -> +inf

There may be even more possibilities, but these examples come to mind right away. Why haven’t these patterns been implemented? It’s particularly puzzling that pattern (b) is folded but not pattern (d). Is it possible that the FoldAdaptor recognizes the commutative nature of these operations and attempts both cases when only one is explicitly written? If so, that would be a helpful trick.
The situation is similar for arith.minimumf.

Re 2, we are adding canonicalization patterns as we need them but we mostly rely on the backend compilers (e.g., LLVM/InstCombine) to do this for us. Some fp canonicalizations are fp-model/FMFs dependent so it’s not only about having the patterns but also about when to apply them. FMFs have been introduced recently so I guess we should be ready to add more :slight_smile:

Because that code was most likely written before arith.minimumf existed :wink:

These ops are quite new. And this answer applies to both parts.

Well, they are just renamed maxf and minf that have been present quite a while, I guess…
So, there is no actual reason not to implement the changes, right? Then, I’m gonna implement them sooner or later.

I know. atomic_rmw was added in February 2020: [MLIR] Add std.atomic_rmw op · llvm/llvm-project@fe210a1 · GitHub. minf/maxf are a much later addition. We didn’t have those for a long time because precisely because we didn’t want to re-encode o/ugt kind of attributes that cmpf has for handling NaNs, so using compare+select was the preferred way.

1 Like