@bondhugula @avarmapml @imaihal could you please review this patch again? Thanks!

@bondhugula Is the current change ok? or should I make any other change? Thanks!

• Use isIdentity() on the layout itself

I don't have. This is my information: "name (email)" : "Tung D. Le (tung@jp.ibm.com)". Thank you!

Thanks @bondhugula! I changed variable names and comments accoring to your comments.

@bondhugula is this patch ok to be landed? Thanks!

@bondhugula @vinayaka-polymage is it the current patch good enough? Thanks!

Thanks, @vinayaka-polymage, for your comments! I simplified the lit tests.

@vinayaka-polymage I updated the patch according to your comments. Thanks!

Move the check of AffineIf into LoopNestStateCollector and use 1D tensors in the lit tests.

@bondhugula I updated the patch to handle only the case that affine.if is inside affine.for. One more test is added for the case where affine.if is sandwiched by the source and dest ops.

Only handle AffineIfOp

Thank you for your comments. I will update the patch to check affine.if only.

Just minor comments.

Done. Thanks!

Edit lit tests according to @dcaballe's comments.

@dcaballe @bondhugula any further comments? Thanks!

LGTM! Thanks!

Hi Diego,

@tungld could you please rebase and fix a conflict due to another revision that updated fusion?

Rebase and fix a conflict

Hi Uday and Diego,

Edit comments and lit tests

I moved the check of defining ops into getFusedLoopNestInsertionPoint and prevented loop nests with values from being fused. Also added TODO for loop nests with values. Thanks!

Move the check into getFusedLoopNestInsertionPoint and prevent fusing loop nests that return values

We should prevent loops with iter_args to be fused

Hi Diego, do you prefer to have it in this patch? I could do it by simply checking the source and dest affine.for, if one of them uses iter_args (or returns a SSA value), we prevent them to be fused.

However, the following code would still fail after the patch, right @tungld?

affine.for %arg2 = 0 to 10 {
  %2 = affine.load %arg1[] : memref<f32>
  affine.store %2, %arg0[%arg2] : memref<10xf32>
}
%0 = affine.load %arg1[] : memref<f32>
%1 = addf %0, %0 : f32
affine.for %arg2 = 0 to 10 {
  %2 = affine.load %arg0[%arg2] : memref<10xf32>
  %3 = divf %1, %2 : f32
}

It worked because we had load in the first loop which has no write-read dependence to %0 = affine.load %arg1[] : memref<f32>. I got this output using this patch:

%0 = affine.load %arg1[] : memref<f32>
%1 = addf %0, %0 : f32
affine.for %arg2 = 0 to 10 {
  %2 = affine.load %arg1[] : memref<f32>
  affine.store %2, %arg0[%arg2] : memref<10xf32>
  %3 = affine.load %arg0[%arg2] : memref<10xf32>
  %4 = divf %1, %3 : f32
}

Thanks for your comments! I just updated the patch.

Reflect reviewers' comments

Check causal dependence

Clean the code

Remove a redundant method

Thank you! Please help me to land it. My "name (email)" : "Tung D. Le (tung@jp.ibm.com)".

@bondhugula any additional modification needed? thanks!

Change the lit test name

Readd the lit test into the new revision

@bondhugula Thank you for your comments! I updated the patch.

Use MemWrite side effect to precisely capture the situation.

Could you let me know what "name <email>" info you want on the commit?

Please use "Tung D. Le <tung@jp.ibm.com>". Thanks!

@bondhugula it seems I don't have permission to push a commit. Could you please help me to land this patch? Many thanks!

Thanks Uday! I updated the code based on your comments.

Add unit tests.

@bondhugula Thank you for your comments! I updated the code based on your comments.

Update the code according to Uday Bondhugula's comments.

Add missing changes for mlir/lib/Transforms/LoopFusion.cpp