Skip to content

Optimize compile times by not skipping allocas#3168

Merged
pow2clk merged 1 commit intomicrosoft:masterfrom
pow2clk:opt_skipalloc
Sep 30, 2020
Merged

Optimize compile times by not skipping allocas#3168
pow2clk merged 1 commit intomicrosoft:masterfrom
pow2clk:opt_skipalloc

Conversation

@pow2clk
Copy link
Copy Markdown
Collaborator

@pow2clk pow2clk commented Sep 30, 2020

Instead of skipping past allocas whenever inserting a new insruction,
which ate up a lot of compilation time, they are inserted at the default
insertion point.

The result is that allocas that would have coalesced just after the
global load and input loads are dispersed throughout the commands. So as
part of dxil finalization, the allocas are moved to the beginning of the
entry block of each function. This results in some minor changes to a
couple tests due to the allocas preceding the loads.

Instead of skipping past allocas whenever inserting a new insruction,
which ate up a lot of compilation time, they are inserted at the default
insertion point.

The result is that allocas that would have coallesced just after the
global load an input loads are dispersed throughout the commands. So as
part of dxil finalization, the allocas are moved to the beginning of the
entry block of each function. This results in some minor changes to a
couple tests due to the allocas preceding the loads.
@pow2clk pow2clk requested review from dmpots and tex3d September 30, 2020 00:10
Copy link
Copy Markdown
Collaborator Author

@pow2clk pow2clk left a comment

Choose a reason for hiding this comment

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

There's some noise here because of the renaming. I've commented on the parts that I think are most relevant. All the renaming is simple and should be entirely unsurprising.

}
}
}

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Besides the simple removal of SkipAllocas, this is the meat of the change. It is required because validation will find that uses of the allocas are not dominated by them if they are left where they are inserted.

It does change the output a bit. Previously, loads of globals and inputs ended up before the allocas. This alteration necessitates the minor test changes.

"getelementptr [4 x float], [4 x float]* %7, i32 0, i32 3",
"getelementptr [4 x float], [4 x float]* %7, i32 0, i32 10",
"getelementptr [4 x float], [4 x float]* %3, i32 0, i32 3",
"getelementptr [4 x float], [4 x float]* %3, i32 0, i32 10",
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Renumbering is a result of the allocas coming first now.

@AppVeyorBot
Copy link
Copy Markdown

@pow2clk pow2clk merged commit 9459577 into microsoft:master Sep 30, 2020
@pow2clk pow2clk deleted the opt_skipalloc branch September 30, 2020 01:11
@pow2clk
Copy link
Copy Markdown
Collaborator Author

pow2clk commented Sep 30, 2020

Hmm. I forgot to add @NicoM1 as a reviewer. 😉

@NicoM1
Copy link
Copy Markdown
Contributor

NicoM1 commented Sep 30, 2020

Hmm. I forgot to add @NicoM1 as a reviewer. 😉

D'aww, hope everyone is surviving launch :D PR looks great!

All the best to the team!

pow2clk pushed a commit that referenced this pull request Oct 5, 2020
pow2clk pushed a commit that referenced this pull request Oct 5, 2020
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