Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Codegen][Common] Add a pass to linearize memrefs #19332

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

Abhishek-Varma
Copy link
Contributor

-- This commit creates a pass to linearize memrefs.
-- The pass iree-linearize-memrefs will be iteratively worked upon to make it an inter-procedural pass.
-- Currently it supports limited operations.

Signed-off-by: Abhishek Varma avarma094@gmail.com

@Abhishek-Varma Abhishek-Varma force-pushed the avarma_linearize_memrefs_pass branch 2 times, most recently from 0fb0475 to 85bdf08 Compare November 28, 2024 06:14
Copy link
Contributor

@MaheshRavishankar MaheshRavishankar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Abhishek! Left a few comments.

@Abhishek-Varma
Copy link
Contributor Author

Hi @MaheshRavishankar @qedawkins - I've addressed the comments. This is ready for re-reviewing. Thanks!

Copy link
Contributor

@krzysz00 krzysz00 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A minor note

@krzysz00 krzysz00 self-requested a review January 16, 2025 23:45
@Abhishek-Varma Abhishek-Varma force-pushed the avarma_linearize_memrefs_pass branch 2 times, most recently from c9dc955 to 20747ec Compare January 17, 2025 08:39
@Abhishek-Varma
Copy link
Contributor Author

Hi @krzysz00 @qedawkins @MaheshRavishankar - I've addressed all comments and the CI seems to be happy now - you may re-review/approve. Thanks!

Copy link
Contributor

@qedawkins qedawkins left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A similar pass came up in a recent discussion, would be good to get a review from @krzysz00 too

}

static FailureOr<SmallVector<OpFoldResult>>
getMixedOrigSize(Location loc, PatternRewriter &rewriter, Value sourceValue) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method could be simpler if you use memref::getMixedSizes. Something like

  MemRefType sourceType = llvm::cast<MemRefType>(sourceValue.getType());
  Operation *sourceOp = sourceValue.getDefiningOp();
  if (!isa<memref::AllocOp, memref::AllocaOp>(sourceOp) && !sourceType.hasStaticShape()) {
    return failure();
  }
  return memref::getMixedSizes(rewriter, sourceValue);

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar reasoning as previous thread.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Quinn is right. We should be able to just use getMixedSizes. I think you dont even need the if condition that he has there.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct. I've responded here.

@krzysz00
Copy link
Contributor

To link what I think we might want to do, #19851

@Abhishek-Varma Abhishek-Varma force-pushed the avarma_linearize_memrefs_pass branch from eb1957f to a37f8a4 Compare February 3, 2025 10:15
-- This commit creates a pass to linearize memrefs.
-- The pass `iree-linearize-memrefs` will be iteratively worked upon
   to make it an inter-procedural pass.
-- Currently it supports limited operations.

Signed-off-by: Abhishek Varma <abhvarma@amd.com>
Signed-off-by: Abhishek Varma <abhvarma@amd.com>
Signed-off-by: Abhishek Varma <abhvarma@amd.com>
Signed-off-by: Abhishek Varma <abhvarma@amd.com>
Signed-off-by: Abhishek Varma <abhvarma@amd.com>
Signed-off-by: Abhishek Varma <abhvarma@amd.com>
Signed-off-by: Abhishek Varma <abhvarma@amd.com>
Signed-off-by: Abhishek Varma <abhvarma@amd.com>
Signed-off-by: Abhishek Varma <abhvarma@amd.com>
Signed-off-by: Abhishek Varma <abhvarma@amd.com>
@Abhishek-Varma Abhishek-Varma force-pushed the avarma_linearize_memrefs_pass branch from a37f8a4 to 67a3439 Compare February 7, 2025 08:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants