[PATCH] D156471: [mlir] Support fast-math friendly constants for identity value

[Renato Golin via Phabricator via llvm-commits]

rengolin added inline comments.


================
Comment at: mlir/include/mlir/Dialect/Arith/IR/Arith.h:147
 Value getIdentityValue(AtomicRMWKind op, Type resultType, OpBuilder &builder,
-                       Location loc);
+                       Location loc, bool useOnlyFiniteValue);
 
----------------
qcolombet wrote:
> rengolin wrote:
> > Can we make these getters to set a default value? then we don't need to override on all calls, only when we know we need, for instance, when the front-end / pass knows what it's doing.
> Yes we could. I decided against it because I wanted people to think about this problem when they use this API.
> Now, since this may be a temporary solution, it may be better to limit the changes.
> 
> WDYT?
> I wanted people to think about this problem when they use this API.

This works for people adding new code, not the existing code that is being changed with this patch, which the original authors are probably not going to look back.

> it may be better to limit the changes.

Also, if the RFC linked goes ahead, the argument will change soon and lead to more mechanical changes.

I'd keep the changes minimal (default value) for now and enter the RFC discussion for the long term strategy.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156471/new/

https://reviews.llvm.org/D156471