[Mlir-commits] [mlir] [mlir][dataflow] Fix for integer range analysis propagation bug (PR #93199)
Krzysztof Drewniak
llvmlistbot at llvm.org
Fri May 24 16:02:19 PDT 2024
https://github.com/krzysz00 commented:
So to keep the discussion going, I remember keeping `ConstantIntRanges` and the lattices separate back in the day so as to make the integer range inference usable for cases other than that one dataflow analysis. But that's not a super hard goal and so this might be a reasonable solution.
Re std::function vs function_ref, I don't have a strong opinion there - all I can say is that the intent was to pass in a transient pointer to a callback. Seems reasonable to make that change.
But also, wrt the interface and backwards-compatibily, how about
1. Provide a new method, possibly with the same name (or if not, something like `inferResultRangesFromOptional`) that takes `ArrayRef<std::optional<ConstantIntRanges>>` instead of `ArrayRef<ConstantIntRanges>`
2. Implement the optional version in terms of the non-optional version by default: that is, if someone doesn't implement `inferResultRanges(ArrayRef<std::optional<ConstantIntRanges>>, SetResultFn)`, it'll loop through the inputs and, if they're all not `none`, call `inferResultRanges()` on the unwrapped values. This gives everyone an upgrade path.
https://github.com/llvm/llvm-project/pull/93199
More information about the Mlir-commits
mailing list