Later, we may think in extending it to other ops or dialects (e.g. index)
I will add IntegerOverflowFlags bitenum and attr to arith dialect, which will be carbon copy of exisitng LLVM dialect IntegerOverflowFlags enum. Default sematics will remain the same (wraparound).
We can potentially reuse same enum and attr between dialects, but I don’t want to add dependency between arith and LLVM (float fastmath flags are implemented in the same way).
Existing canonicalizations/transformations which are not aware of those flags may drop those flags arbitrarily. We will update them gradually as needed, as dropping these flags will only lead to missing optimizations and not to correctness issues.
Lowering
For LLVM those flags will be directly lowered to nsw/nuw flags
For SPIR-V they will be lowered to NoSignedWrap/NoUnsignedWrap op decorations.
If some backend doesn’t support such flags it can safely ignore them and assume default (wraparound) sematics.
Thanks for writing this up @Hardcode84, the proposal makes sense to me.
Should we also allow these on vector/gpu reduction ops, like we do with fast math flags? In the SPIR-V lowering path, we would need these to avoid UB when emitting some integer dot product ops like spirv.SDotAccSat.
Yeah making this incremental makes sense for me but I would like to make sure we can get to the full solution eventually. As for reduction ops and flags, I wouldn’t mind having different enum values for int/float reductions so that they map 1:1 to arith ops – I think it’ll be just a bit more code but simpler logic (no need to branch on the value type) and easier to look up the semantics. But that’s probably a separate discussion.
Sorry, I wasn’t clear, reduce ops like vector.reduction support both int and float types, so they will have both sets of flags attached as 2 different args. Is this ok, or do we want to split reduction ops into separate int/float versions?
I’d think we allow both attributes (fast math and overflow, as defined in arith) in the assembly format and then add check in the verifier that the op doesn’t mix fp/int types with int/fp attributes.
I took some code from your impl, but made canonicalizations to just drop flags. It’s quite annoying to deal with tablegen rewrites and I’ll update them in separate PR.
Also, had to include LLVM translation code in this PR, as existing patterns will crash when unexpected attributes are encountered.
Just a suggestion for the overall execution strategy: could you start a tracking issue on github with all the planned integer overflow tasks? This should be a lightweight mechanism to track the progress and keep track of all the related PRs
I think we can support the integer overflow flags for complex dialect as we do (still on-doing) to support the fastmath flag in converting to arith and LLVM dialect. It allows us to keep the overflow flags semantics from the complex dialect to the lower dialect in the same manner. What do you think about it? If it makes sense, I’ll try to work on that.
There is a task for canonicalization in the above list. Does it include not only fixing missing flags in canonicalization but also adding more flags to operations?
I’m working on Flang and it’s found that some nsw flags on arith.addi can help optimization. I’m roughly thinking of implementing the feature adding nsw flags in canonicalizer, but I’m not sure this conforms to the design of arith dialect considering the following discussion.
