[Mlir-commits] [mlir] [MLIR][Arith] Add denormal attribute to binary/unary operations (PR #112700)

Mehdi Amini llvmlistbot at llvm.org
Mon Dec 9 09:20:14 PST 2024


joker-eph wrote:

> @joker-eph, while I agree with your general point and the need for a charter, your comment is not specific enough to warrant a reversal. Perhaps you could be a bit more specific in a forum thread and we continue from there?

My comment is that without being able to clearly map this change to the "charter" for arith, we can't just land this as-is without proper RFC/discussion. We're back to:

> Generally, changes to core dialects like arith should really go through an RFC on the forum before being a PR. 

The fact that you missed this discussion is a good indication that this didn't get enough visibility.

In this case my concerns about this change and the associated tradeoffs to consider, warrants a revert IMO. 
This is in line with [LLVM developer policy](https://llvm.org/docs/DeveloperPolicy.html#patch-reversion-policy) and past practices:

> - Remember, it is normal and healthy to have patches reverted. Having a patch reverted does not necessarily mean you did anything wrong.
> - Any time you learn of a serious problem with a change, you should revert it. 
> - If you are asked to revert by another contributor, please revert and discuss the merits of the request offline (unless doing so would further destabilize tip of tree).



https://github.com/llvm/llvm-project/pull/112700


More information about the Mlir-commits mailing list