This is an archive of the discontinued LLVM Phabricator instance.

[MLIR] Simplify affine.if having yield values and trivial conditions
ClosedPublic

Authored by srishti-pm on Jul 4 2021, 11:19 PM.

Details

Summary

When an affine.if operation is returning/yielding results and has a
trivially true or false condition, then its 'then' or 'else' block,
respectively, is promoted to the affine.if's parent block and then, the
affine.if operation is replaced by the correct results/yield values.
Relevant test cases are also added.

Signed-off-by: Srishti Srivastava <srishti.srivastava@polymagelabs.com>

Diff Detail

Event Timeline

srishti-pm created this revision.Jul 4 2021, 11:19 PM
srishti-pm requested review of this revision.Jul 4 2021, 11:19 PM

Thanks for adding this simplification.

mlir/test/Dialect/Affine/simplify-affine-structures.mlir
403

3 -> Three

407–411

Can we make this set simpler since the goal here is to test affine.if simplification in the presence of yield values?

srishti-pm updated this revision to Diff 356501.Jul 5 2021, 7:16 AM
srishti-pm marked 2 inline comments as done.

[MLIR] Simplify affine.if having yield values and trivial conditions

When an affine.if operation is returning/yielding results and has a
trivially true or false condition, then its 'then' or 'else' block,
respectively, is promoted to the affine.if's parent block and then, the
affine.if operation is replaced by the correct results/yield values.
Relevant test cases are also added.

Signed-off-by: Srishti Srivastava <srishti.srivastava@polymagelabs.com>

Differential Revision: https://reviews.llvm.org/D105418

bondhugula accepted this revision.Jul 5 2021, 11:02 AM

LGTM! Just a few minor comments.

mlir/test/Dialect/Affine/simplify-affine-structures.mlir
391

oldO1 -> zero? More readable below this way.

406

Nit: Else within backticks.

409–410

For readability, you can name these captured variables like c7, c13, etc. Here and below - this is the typical convention.

479–488

All test cases appear to have block local defined constant ops being yielded. Could you just have one where a value defined outside the affine.if is being yielded and you have that block being promoted?

This revision is now accepted and ready to land.Jul 5 2021, 11:02 AM
srishti-pm updated this revision to Diff 356599.Jul 5 2021, 9:52 PM
srishti-pm marked 3 inline comments as done.

Modified test cases

pr4tgpt accepted this revision.Jul 5 2021, 10:12 PM

LGTM!

srishti-pm marked an inline comment as done.Jul 6 2021, 12:35 AM

Address comments

bondhugula accepted this revision.Jul 6 2021, 3:44 AM
srishti-pm added a comment.EditedJul 7 2021, 1:57 AM

@mehdi_amini, could you kindly change my Phabricator username to "srishti-pml", which is also my GitHub username attached to this account now. I am unable to do it myself because when I try, a pop-up comes and it says "Only administrators can change usernames".

xgupta added a subscriber: xgupta.EditedJul 9 2021, 4:45 AM
In D105418#2861432, @SrishtiSrivastavaPolyMage wrote:

@mehdi_amini, could you kindly change my Phabricator username to "srishti-pml", which is also my GitHub username attached to this account now. I am unable to do it myself because when I try, a pop-up comes and it says "Only administrators can change usernames".

You should simply create a new account with your @polymagelabs.com email address and get a new username srishti-pml otherwise simply change your GitHub username to SrishtiSrivastavPolyMage if you only want consistency.

In D105418#2861432, @SrishtiSrivastavaPolyMage wrote:

@mehdi_amini, could you kindly change my Phabricator username to "srishti-pml", which is also my GitHub username attached to this account now. I am unable to do it myself because when I try, a pop-up comes and it says "Only administrators can change usernames".

You should simply create a new account with your @polymagelabs.com email address and get a new username srishti-pml otherwise simply change your GitHub username to SrishtiSrivastavPolyMage if you only want consistency.

Hi, actually I want my username in Phabricator to be srishti-pml. So, when I tried changing my Phabricator username, I found out that I don't have the required permissions.

Now, my Phabricator account is associated with my GitHub account, in which my username was SrishtiSrivastavaPolymage before. So, I changed my GitHub username to srishti-pml, hoping that it would change by Phabricator username. But my Phabricator username didn't change. That is why I am requesting @mehdi_amini to kindly change my Phabricator username to srishti-pml.

Hi, actually I want my username in Phabricator to be srishti-pml.

I said you to create new account.

So, when I tried changing my Phabricator username, I found out that I don't have the required permissions.

Now, my Phabricator account is associated with my GitHub account, in which my username was SrishtiSrivastavaPolymage before. So, I changed my GitHub username to srishti-pml, hoping that it would change by Phabricator username. But my Phabricator username didn't change. That is why I am requesting @mehdi_amini to kindly change my Phabricator username to srishti-pml.

I have no problem. But actually I think _volunteer_ phabricator administrator are not responsible for changing usernames of 20000+ llvm phab usrers or do they?
It is usually difficult to entertain such request :)

Hi, actually I want my username in Phabricator to be srishti-pml.

I said you to create new account.

New accounts have the disadvantage of losing the history of the account. Also if you're using GitHub login, you can't just have a new account (I think?).

So, when I tried changing my Phabricator username, I found out that I don't have the required permissions.

Now, my Phabricator account is associated with my GitHub account, in which my username was SrishtiSrivastavaPolymage before. So, I changed my GitHub username to srishti-pml, hoping that it would change by Phabricator username. But my Phabricator username didn't change. That is why I am requesting @mehdi_amini to kindly change my Phabricator username to srishti-pml.

I have no problem. But actually I think _volunteer_ phabricator administrator are not responsible for changing usernames of 20000+ llvm phab usrers or do they?
It is usually difficult to entertain such request :)

It takes ~10s to do, and it is really not common (it happened a handful of times per year so far).

Awesome, Thanks!

Hi, actually I want my username in Phabricator to be srishti-pml.

I said you to create new account.

New accounts have the disadvantage of losing the history of the account. Also if you're using GitHub login, you can't just have a new account (I think?).

So, when I tried changing my Phabricator username, I found out that I don't have the required permissions.

Now, my Phabricator account is associated with my GitHub account, in which my username was SrishtiSrivastavaPolymage before. So, I changed my GitHub username to srishti-pml, hoping that it would change by Phabricator username. But my Phabricator username didn't change. That is why I am requesting @mehdi_amini to kindly change my Phabricator username to srishti-pml.

I have no problem. But actually I think _volunteer_ phabricator administrator are not responsible for changing usernames of 20000+ llvm phab usrers or do they?
It is usually difficult to entertain such request :)

It takes ~10s to do, and it is really not common (it happened a handful of times per year so far).

Thanks for the help, @mehdi_amini