[llvm] Update foldFMulReassoc to respect absent fast-math flags (PR #88589)
Andy Kaylor via llvm-commits
llvm-commits at lists.llvm.org
Tue Apr 16 13:06:21 PDT 2024
================
@@ -636,26 +636,43 @@ Instruction *InstCombinerImpl::foldFMulReassoc(BinaryOperator &I) {
// expression.
if (match(Op1, m_Constant(C)) && C->isFiniteNonZeroFP()) {
Constant *C1;
- if (match(Op0, m_OneUse(m_FDiv(m_Constant(C1), m_Value(X))))) {
+ if (match(Op0,
+ m_AllowReassoc(m_OneUse(m_FDiv(m_Constant(C1), m_Value(X)))))) {
----------------
andykaylor wrote:
I thought about this a bit. One possibility would be to add something like
```
FastMastFlags RequiredFMF = FastMathFlags::AllowReassoc;
...
m_fmf_FDiv(RequiredFMF, m_Value(X), m_Value(Y))
```
That's no less verbose than the current method if you only want to check reassoc, but it would allow us to check other flags too. Is this what you had in mind?
The thing I'm not happy about with either the current implementation or the suggestion above is that it isn't well suited to retrieving the fast-math flags for any given operation, which is needed when combining operations. You can see this in the update I posted to handle the `(X / Y) * Z --> (X * Z) / Y` case. In that code, we're doing a commutative match on the fmul operands, and I had to deduce which operand was the fdiv it matched and extract the flags, Maybe something like this would be better?
```
FastMastFlags DivFMF;
...
m_fmf_FDiv(m_reassoc(m_FMF(DivFMF)), m_Value(X), m_Value(Y))
```
The m_FMF() would retrieve the actual flags used, and m_reassoc() applies a check during the matching.
https://github.com/llvm/llvm-project/pull/88589
More information about the llvm-commits
mailing list