Page MenuHomePhabricator

NFC - Moved AffineOps IR / Transforms
ClosedPublic

Authored by rsuderman on Fri, Mar 13, 1:55 PM.

Details

Summary

NFC - Moved AffineOps related transforms

Change AffineOps Dialect structure to better group both IR and Tranforms. This included extracting transforms directly related to AffineOps. Also move AffineOps to Affine.

Diff Detail

Event Timeline

rsuderman created this revision.Fri, Mar 13, 1:55 PM

I think this has a lot of conflict potential with a revision by @ftynse, maybe check with him?

rsuderman updated this revision to Diff 250324.Fri, Mar 13, 4:32 PM

Updated CMake build to pass.

Updating D76161: Moved AffineOps IR / Transforms

Change AffineOps Dialect structure to better group both IR and Tranforms. This
included extracting transforms direclty related to AffineOps

rsuderman updated this revision to Diff 250325.Fri, Mar 13, 4:33 PM

Typo

Updating D76161: Moved AffineOps IR / Transforms

Change AffineOps Dialect structure to better group both IR and Tranforms. This
included extracting transforms direclty related to AffineOps

I think this has a lot of conflict potential with a revision by @ftynse, maybe check with him?

I'll make sure to loop him in on the review.

rsuderman retitled this revision from Moved AffineOps IR / Transforms Change AffineOps Dialect structure to better group both IR and Tranforms. This included extracting transforms direclty related to AffineOps to Moved AffineOps IR / TransformsChange AffineOps Dialect structure to better group both IR and Tranforms. Thisincluded extracting transforms direclty related to AffineOps.Fri, Mar 13, 4:34 PM
mehdi_amini added inline comments.Fri, Mar 13, 5:11 PM
mlir/test/lib/Transforms/TestVectorizationUtils.cpp
16

Since you had to change all of them, and since every downstream user will have to adjust, what about also renaming the mlir/Dialect/AffineOps into mlir/Dialect/Affine ?

rsuderman updated this revision to Diff 250338.Fri, Mar 13, 6:21 PM

clang-format

bondhugula requested changes to this revision.Fri, Mar 13, 9:03 PM
bondhugula added a reviewer: bondhugula.
bondhugula added a subscriber: bondhugula.

Please see this discussion started by @ftynse
https://llvm.discourse.group/t/rfc-canonical-file-paths-to-dialects/621
If we are doing a move, we might as well get the directory names renamed consistently. AffineOps -> Affine

bondhugula requested changes to this revision.Fri, Mar 13, 9:04 PM
This revision now requires changes to proceed.Fri, Mar 13, 9:04 PM

@rsuderman @mehdi_amini @bondhugula the renaming AffineOps -> Affine is part of the conflicting changes @ftynse has in flight and was holding off on submitting because internal integration would be impacted.

@rsuderman @mehdi_amini @bondhugula the renaming AffineOps -> Affine is part of the conflicting changes @ftynse has in flight and was holding off on submitting because internal integration would be impacted.

Its been over a week, that isn't great justification. I'm pretty sure he's just on vacation.

mlir/include/mlir/InitAllPasses.h
29

Duplicate?

rsuderman marked an inline comment as done.

Move AffineOps to Affine

rsuderman marked an inline comment as done and an inline comment as not done.Mon, Mar 16, 11:04 AM

Should be updated for comments.

mlir/include/mlir/InitAllPasses.h
29

I don't see a duplicate? AffineOps/Passes.h was newly added in this CL.

mlir/test/lib/Transforms/TestVectorizationUtils.cpp
16

Moved AffineOps to Affine.

rsuderman retitled this revision from Moved AffineOps IR / TransformsChange AffineOps Dialect structure to better group both IR and Tranforms. Thisincluded extracting transforms direclty related to AffineOps to NFC - Moved AffineOps IR / Transforms.Mon, Mar 16, 11:21 AM
rsuderman edited the summary of this revision. (Show Details)
rriddle accepted this revision.Mon, Mar 16, 11:52 AM
rriddle marked an inline comment as done.
rriddle added inline comments.
mlir/include/mlir/Dialect/Affine/EDSC/Intrinsics.h
7–8

Can you update each of these header guard comments now?

mlir/include/mlir/Dialect/Affine/Passes.h
10

AffineOps -> Affine

mlir/include/mlir/InitAllPasses.h
29

It was previously showing + #include "mlir/Dialect/FxpMathOps/Passes.h"

Update for rriddle@ comments and synced to head.

rsuderman marked 5 inline comments as done.Mon, Mar 16, 12:26 PM

Addressed rriddle@ comments and synced to head.

rsuderman updated this revision to Diff 250621.Mon, Mar 16, 1:14 PM

git clang-format

mravishankar resigned from this revision.Tue, Mar 17, 12:46 PM
rsuderman updated this revision to Diff 250921.Tue, Mar 17, 3:00 PM

Sync to head and reupdate.

Can you sync to head?

rsuderman updated this revision to Diff 251492.Thu, Mar 19, 3:43 PM

Sync to head

bondhugula accepted this revision.Thu, Mar 19, 6:41 PM
This revision is now accepted and ready to land.Thu, Mar 19, 6:41 PM

Synced to head.

This revision was automatically updated to reflect the committed changes.