Page MenuHomePhabricator

[MLIR][Affine] Remove assert that prevents 0-dim affine.parallel loops
AbandonedPublic

Authored by flaub on Apr 19 2021, 11:58 AM.

Details

Summary

It's legal to have affine.parallel ops that don't have any IVs specified.

Diff Detail

Event Timeline

flaub created this revision.Apr 19 2021, 11:58 AM
flaub requested review of this revision.Apr 19 2021, 11:58 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 19 2021, 11:58 AM
bondhugula requested changes to this revision.Apr 19 2021, 1:43 PM

Sounds good, but a test case is missing here. (You could set up one with -canonicalize which would set the lower/upper bounds in an attempt to simplify?). Separately, are there test cases with 0-d affine.parallel ops especially with -lower-affine to ensure they correctly lower to a similar scf.parallel or just the body?

This revision now requires changes to proceed.Apr 19 2021, 1:43 PM
flaub added a comment.Thu, May 20, 7:16 PM

@bondhugula Sorry I haven't touched this in a long time. I've failed to come up with a test case that will exercise this code path with the existing passes/canonicalizations that exist in MLIR itself. This patch however is important for many of our downstream passes that we do plan to submit to MLIR once it becomes more mature and we've done a review on the forum.

Do you think it's worth the trouble to write unit tests for this case?

flaub added a comment.Thu, May 20, 7:17 PM

I believe that when I first wrote the setLowerBounds and setUpperBounds methods, I copy/pasted them largely from the affine.for op. This is why I think those assertions exist currently, but they were never intended in our original design of the affine.parallel op.

flaub abandoned this revision.Wed, May 26, 4:46 PM