[Mlir-commits] [mlir] [mlir][SCF] Deprecate `linalg::tileToForallOp` and `linalg::tileToForallOpUsingTileSizes` (PR #91878)

[llvmlistbot at llvm.org]

MaheshRavishankar wrote:

> > I dont know what the original implementation was doing, and where it was going wrong. I didnt try to fix the bug, but the refactoring showed a change in the lit test and then I verified that the current PR is doing the right thing and the old path is wrong. I dont see much value in triaging a bug on a path that is being deleted.
> 
> Help me understand this better: you started refactoring things, then found a bug by some other means, changed behavior, now it is correct in your opinion but cannot pinpoint what was wrong.

As far as I know I didn't change any behavior. I didn't _try_ to fix a bug. I didn't even know there was a bug. I was trying to see differences in the lit test and I think the output of the lit test with the refactoring is correct. What was there before was wrong as far as I can tell. 

> 
> Please elaborate on how the bug showed up, was it some IREE execution test ? is this something that did not show before this refactoring or was it always there ? Or is it just "the IR changed" when refactoring happened.
> 
> In any case, the MO here can be improved.
> I agree there is little to no value in fixing the bug before the refactoring. 
> My opinion is there is strong negative value in conflating a material change we don't understand with a deep refactoring.
>

> Your path forward here is to land the refactoring without changes to the affine.max part and then to address the bug separately after we understand what is going wrong and have a proper test for it.

There is no change to affine.max. This should be just refactoring. What you are suggesting is essentially I find the bug and introduce it back with the refactoring? Please take a look at the change in the lit tests (which are relatively minor). The changed tests are correct. I have no idea why the old path did something wrong, but it seems to be. And like you also agree above there is no value in triaging that.



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