[Mlir-commits] [mlir] [mlir][affine][Analysis] Add conservative bounds for semi-affine mods (PR #93576)

[Kunwar Grover]

Groverkss wrote:

> > This is not really my area of expertise, but looks like all comments from @Groverkss have been addressed. Kunwar, can you take another look? This LGTM :)
> > > Just looking in the affine sources I found:
> > > ```
> > >   /// RHS of mod is always a constant or a symbolic expression with a positive
> > >   /// value.
> > > ```
> > > 
> > > 
> > >     
> > >       
> > >     
> > > 
> > >       
> > >     
> > > 
> > >     
> > >   
> > > So does that mean if the (symbolic) rhs is negative for a semi-affine mod it's UB?
> > 
> > 
> > Do you have a link? Not supporting it might be safer - it's not obvious to me why it would be UB TBH.
> 
> https://github.com/llvm/llvm-project/blob/main/mlir/include/mlir/IR/AffineExpr.h#L45-L46. And I mean UB according to the affine dialect. If it's specified that the symbolic RHS is always positive, then it's not really defined for the negative case (which is still possible for symbolic expressions). If the negative case is not defined/intended to be supported that makes things easier for us (since our assumption that the `RHS > 0` would always hold for correct code).

If it's in AffineExpr definition, I would say this is part of AffineExpr, not affine dialect. I think it's ok to assume it and link it. If it is later removed from it's definition, we can remove this constraint (and add a parameter to specify if rhs is positive)

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