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

Consider data descriptor offset when propagating memlets #1461

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

lukastruemper
Copy link
Contributor

@lukastruemper lukastruemper commented Nov 29, 2023

This PR adds the data descriptor's offsets to the memlet propagation.

@lukastruemper
Copy link
Contributor Author

@Sajohn-CH I saw the same issue in one of my fortran examples and found a fix. I added your code as a new tests.

#1372

@lukastruemper lukastruemper marked this pull request as draft November 29, 2023 20:48
@lukastruemper lukastruemper marked this pull request as ready for review November 30, 2023 09:34
@tbennun tbennun linked an issue Nov 30, 2023 that may be closed by this pull request
@tbennun
Copy link
Collaborator

tbennun commented Nov 30, 2023

@acalotoiu I thought we were removing/deprecating offsets?

@tbennun tbennun requested review from acalotoiu and removed request for tbennun November 30, 2023 16:19
@lukastruemper
Copy link
Contributor Author

@acalotoiu I thought we were removing/deprecating offsets?

Lex explained that they will solve it in the frontend. I simply didn‘t know that we also deprecate it. Then we could close the PR

@acalotoiu
Copy link
Contributor

@acalotoiu I thought we were removing/deprecating offsets?

Yes - but while we intend to do that, actually removing them is not an engineering effort planned for the short or medium term. I propose we allow the fix for the current state for now.

Copy link
Contributor

@acalotoiu acalotoiu left a comment

Choose a reason for hiding this comment

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

LGTM

@lukastruemper
Copy link
Contributor Author

LGTM

@tbennun merge?

@tbennun tbennun added this pull request to the merge queue Jan 7, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jan 7, 2024
@tbennun tbennun added the in the merge queue waiting for CI to work again label Jan 14, 2024
JanKleine added a commit to JanKleine/dace that referenced this pull request Feb 14, 2024
@alexnick83 alexnick83 enabled auto-merge (squash) February 20, 2024 16:16
@alexnick83 alexnick83 added this pull request to the merge queue Feb 20, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to no response for status checks Feb 21, 2024
@alexnick83 alexnick83 added this pull request to the merge queue Feb 21, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to no response for status checks Feb 21, 2024
@alexnick83 alexnick83 added this pull request to the merge queue Feb 21, 2024
@phschaad phschaad removed this pull request from the merge queue due to the queue being cleared Feb 22, 2024
@alexnick83 alexnick83 added this pull request to the merge queue Feb 23, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Feb 23, 2024
@phschaad phschaad added this pull request to the merge queue Feb 23, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Feb 23, 2024
@alexnick83 alexnick83 added this pull request to the merge queue Feb 23, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Feb 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in the merge queue waiting for CI to work again
Projects
None yet
Development

Successfully merging this pull request may close these issues.

LoopToMap followed by simplify leads to out-of-bounds memlets
4 participants