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.

Sign up for GitHub

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

Jump to bottom

[MLIR] Update lhlo.const to linalg lowering to use affine.store inste… #40925

Merged
merged 1 commit into from
Jul 10, 2020
Merged

[MLIR] Update lhlo.const to linalg lowering to use affine.store inste… #40925

merged 1 commit into from
Jul 10, 2020

Conversation

bondhugula
Copy link
Contributor

…ad of std.store

The xla_lhlo.const lowering uses std.store to store a constant to
0-d memrefs. Update it to affine.store since such an access is trivially
affine (no indices). An affine.store can always be lowered to std.store.

@google-ml-butler google-ml-butler bot added the size:S CL Change Size: Small label Jun 30, 2020
@googlebot
Copy link

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

@joker-eph
Copy link
Contributor

@bondhugula the CLA-related message is in general an indication that the commit email isn't the usual one

@joker-eph joker-eph requested review from pifon2a and removed request for joker-eph June 30, 2020 03:13
@googlebot
Copy link

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

@bondhugula
Copy link
Contributor Author

@googlebot I fixed it.

@bondhugula
Copy link
Contributor Author

@bondhugula the CLA-related message is in general an indication that the commit email isn't the usual one

Thanks, fixed.

@gbaned gbaned self-assigned this Jun 30, 2020
@gbaned gbaned added this to Assigned Reviewer in PR Queue via automation Jun 30, 2020
@bondhugula
Copy link
Contributor Author

Ping @pifon2a - thanks.

PR Queue automation moved this from Assigned Reviewer to Approved by Reviewer Jul 2, 2020
pifon2a
pifon2a previously approved these changes Jul 2, 2020
View reviewed changes
@google-ml-butler google-ml-butler bot added kokoro:force-run Tests on submitted change ready to pull PR ready for merge process labels Jul 2, 2020
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Jul 2, 2020
@gbaned
Copy link
Contributor

gbaned commented Jul 7, 2020

@bondhugula Can you please resolve conflicts? Thanks!

@gbaned gbaned removed the ready to pull PR ready for merge process label Jul 7, 2020
@bondhugula
Copy link
Contributor Author

@bondhugula Can you please resolve conflicts? Thanks!

Done, thanks!

@bondhugula bondhugula force-pushed the lhlo_to_linalg_affine_store branch from 89db989 to 3ec00d7 Compare July 7, 2020 16:31
PR Queue automation moved this from Approved by Reviewer to Reviewer Requested Changes Jul 7, 2020
@gbaned
Copy link
Contributor

gbaned commented Jul 9, 2020

@bondhugula Still, conflicts appearing, can you please resolve those? Thanks!

@gbaned gbaned added the stat:awaiting response Status - Awaiting response from author label Jul 9, 2020
@bondhugula
[MLIR] Update lhlo.const to linalg lowering to use affine.store inste…
9e18ede 
…ad of std.store

The xla_lhlo.const lowering uses std.store to store a constant to
0-d memrefs. Update it to affine.store since such an access is trivially
affine (no indices). An affine.store can always be lowered to std.store.
@bondhugula bondhugula force-pushed the lhlo_to_linalg_affine_store branch from 3ec00d7 to 9e18ede Compare July 9, 2020 17:20
@bondhugula
Copy link
Contributor Author

@bondhugula Still, conflicts appearing, can you please resolve those? Thanks!

Done - now. (Looks like those conflicts happened in the time my rebased commit was tested.)

PR Queue automation moved this from Reviewer Requested Changes to Approved by Reviewer Jul 10, 2020
@google-ml-butler google-ml-butler bot added kokoro:force-run Tests on submitted change ready to pull PR ready for merge process labels Jul 10, 2020
@gbaned gbaned added kokoro:force-run Tests on submitted change ready to pull PR ready for merge process and removed kokoro:force-run Tests on submitted change ready to pull PR ready for merge process stat:awaiting response Status - Awaiting response from author labels Jul 10, 2020
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Jul 10, 2020
@tensorflow-copybara tensorflow-copybara merged commit 0e8ef97 into tensorflow:master Jul 10, 2020
PR Queue automation moved this from Approved by Reviewer to Merged Jul 10, 2020
joker-eph pushed a commit to tensorflow/mlir-hlo that referenced this pull request Jul 30, 2020
@bondhugula @joker-eph
PR #40925: [MLIR] Update lhlo.const to linalg lowering to use affine.…
d166b66 
…store inste…

Imported from GitHub PR tensorflow/tensorflow#40925

…ad of std.store

The xla_lhlo.const lowering uses std.store to store a constant to
0-d memrefs. Update it to affine.store since such an access is trivially
affine (no indices). An affine.store can always be lowered to std.store.
Copybara import of the project:

--
9e18ede72fbbca107177bd742921e4cbf77adc82 by Uday Bondhugula <uday@polymagelabs.com>:

[MLIR] Update lhlo.const to linalg lowering to use affine.store instead of std.store

The xla_lhlo.const lowering uses std.store to store a constant to
0-d memrefs. Update it to affine.store since such an access is trivially
affine (no indices). An affine.store can always be lowered to std.store.

COPYBARA_INTEGRATE_REVIEW=tensorflow/tensorflow#40925 from polymage-labs:lhlo_to_linalg_affine_store 9e18ede72fbbca107177bd742921e4cbf77adc82
PiperOrigin-RevId: 320623152
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@joker-eph joker-eph joker-eph approved these changes

@pifon2a pifon2a pifon2a left review comments

Assignees

@gbaned gbaned

Labels
cla: yes ready to pull PR ready for merge process size:S CL Change Size: Small
Projects
PR Queue
  
Merged
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@bondhugula @googlebot @joker-eph @gbaned @pifon2a @kokoro-team @tensorflow-copybara