This is an archive of the discontinued LLVM Phabricator instance.

[mlir] Add the AffineScope trait to linalg.generic ops
AbandonedPublic

Authored by agostini01 on Mar 16 2021, 7:32 PM.

Details

Summary

This change enables using affine ops inside the body of a
linalg.indexed_generic operation.

This is in response to the discussion: https://llvm.discourse.group/t/should-linalg-indexed-generic-allow-for-affine-operations-on-its-body/2889

With the additional of AffineScope the following test started failing:

  • mlir/test/Dialect/Linalg/reshape_fusion.mlir
  • It fails because it swaps index for symbols on generated affine_map
  • Happens only on -linalg-fusion-for-tensor-ops

Diff Detail

Event Timeline

agostini01 created this revision.Mar 16 2021, 7:32 PM
agostini01 requested review of this revision.Mar 16 2021, 7:32 PM
agostini01 abandoned this revision.Mar 16 2021, 7:34 PM

First time using arc. Uploaded the patch with a missing commit. I will create a new patch.

Added the missing commit

agostini01 retitled this revision from [mlir] Implement Lowring of affine.if with results to scf.if to [mlir] Add the AffineScope trait to linalg.generic ops.Mar 16 2021, 7:45 PM
agostini01 edited the summary of this revision. (Show Details)
agostini01 edited the summary of this revision. (Show Details)
agostini01 edited the summary of this revision. (Show Details)Mar 16 2021, 7:47 PM

Added Uday to this patch.

Note, the current changes still cause mlir/test/Dialect/Linalg/reshape_fusion.mlir to fail.
I will need a bit of help debugging it.

Sorry for the convoluted number of actions, it is my first patch with arc.

Remove affine.if to scf.if lowering from this revision

Update the patch

agostini01 edited the summary of this revision. (Show Details)Mar 17 2021, 4:15 PM
nicolasvasilache requested changes to this revision.Mar 24 2021, 7:57 AM
nicolasvasilache added inline comments.
mlir/test/Dialect/Linalg/affine.mlir
191

In-place transpose is incompatible with "parallel" semantics.
This would be a good candidate to evolve the semantics and allow a "permutable band" in linalg but changes are more profound than this revision.

You'll need another example.

This revision now requires changes to proceed.Mar 24 2021, 7:57 AM
agostini01 abandoned this revision.May 19 2021, 1:41 PM

I am abandoning this request since we did not use this functionality in our examples (which mostly consisted of linear algebra exercises that required padding). A simpler approach was to split the problems in 2 parts (padding and compute) which avoided the use of ifs and elses in the inner loops of the algorithm.

mlir/include/mlir/Dialect/Linalg/IR/LinalgStructuredOps.td
508 ↗(On Diff #331147)

This causes mlir/test/Dialect/Linalg/reshape_fusion.mlir to fail, by generating an affine_map with symbols instead of dims.

Expected:

#map7 = affine_map<(d0, d1) -> (d0 + d1 * 4)
//...
%5 = affine.apply #map7(%arg3, %arg2)

What is generated:

#map7 = affine_map<()[s0, s1] -> (s0 * 4 + s1)
//...
%5 = affine.apply #map7()[%arg2, %arg3]
mlir/test/Dialect/Linalg/affine.mlir
191

I was not thinking about the parallel execution implications and I see that this could generate dataraces. I will create a better example and share here in the coming days. Thank you for the comment.