Adding why the issue was not discovered till now

Better commit summary and message

Looks great, thanks for fixing this hard and non-trivial bug !

Please change the commit title to a more relevant one, I am approving this.

@sumesh13 Thanks for addressing all the comments, the change looks good now. Can you please check these last two comments before I approve this ?

@sumesh13 This new generic fix looks good, it handles more cases that were previously not handled. Thanks for this. I have added some comments related to code organization, please check them. Logic looks good to me.

Minor suggestion.

Thank you for bringing up this issue in LICM, it is an important one. I have a few comments on the fix:

1. Inherent issue is definedOps ( going to be renamed to opsWithUsers) is being populated only under some conditions, and does not actually do the job of collecting all ops that define an SSA value in that block.
2. Even after this fix, only AffineForOps will be added to the list, but there are other cases like AffineDma... ops, AffineIfOps that are actually NOT code invariant etc. which will still cause similar problems.

Recent suggestions incorporated. I have asked for clarification on one comment.

Addressing minor comments.

Addressing recent comments, minor.

I have addressed the remaining comments:

1. Instead of not printing the slice at all if it is incorrect, this change prints them but with a remark : "Incorrect slice" during testing.
2. Fast check is being done, and is reusing the bounds checking utility function isSliceMaximalFastCheck. I have verified that with today's test file loop-fusion.mlir, of 156 calls to isSliceValid, 107 calls are returned after the this fast check itself.

Added back the slice computation remarks in tests, marked them incorrect slices.
Added a fast check, reused the utility - isSliceMaximalFastCheck.

Thanks for the review and suggestions !! I have completed the easy ones, and I will continue exploring the pending ones.

Addressing easy comments and suggestions, some important ones
still pending.

If this change/approach is acceptable, we can also change ComputationSliceState::isMaximal() to use the set difference the other way (source - slice), reusing these computations.

Addressing the getOperandTypes comment

Thanks a lot for the insights @bondhugula and @dcaballe ! No more changes needed for this patch then.

Looks good, thanks !

Suggested additional checks and test-cases submitted at https://reviews.llvm.org/D97252

Test case fix.