Open jbarczak opened 7 years ago
I agree that such an option would be useful if not necessary. I think your point about forward compatibility is especially compelling. @dneto0 suggested something very similar in recent correspondence. I will put forward a more specific proposal shortly.
In the meantime, I would recommend looking at this white paper: https://www.lunarg.com/wp-content/uploads/2017/08/SPIR-V-Shader-Size-Reduction-Using-spirv-opt_v1.0.pdf
It contains an option "recipe" which should be generally useful. It also contains descriptions of each of the passes so that users have the information to change the recipe to what is most useful for them.
This white paper is also now pointed at by the SPIRV-Tools README.
Yes, this is a huge issue.
First, i dont see why opengl couldnt have had something similiar - vendor independent glsl translator to whatever bytecode and optimizer.
Second, how the fuck are you supposed to write some spirv bytecode directly ? There must be some human readable language, like glsl or hlsl, that later some compiler converts to spirv. So, vulkan havent invented anything new here, they are just trying to fix the holes of opengl, plus they are still dragging the tail of opengl, because this converter is not 100% compatible, all i found about it is that it is "experimental".
Third, i really would like to see vulkan getting big, but khronos is tripping over themselves on the plain road. It definitely looks like vulkan wasnt meant to be opengl improvement, but some offscreen renderer for opencl, because i couldnt really find anything that vulkan improves over opengl, and found only stuff, that makes it worse compared to opengl...
I second @jbarczak 's comments, the optimizer provides hints for optimal usage of the tool in the header file and the document provided by @greg-lunarg, but is by no means straightforward. I reordered passes by trial and error and it was quite a lengthy process, as some passes behave better than others in certain circumstances, some crash if swapped in some cases, etc. Not very intuitive.
@startas
First, i dont see why opengl couldnt have had something similiar
OpenGL carries a lot of legacy in this regard. Only recently spir-v support was introduced. OpenGL is really old and made some retrospectively bad decisions like forcing each vendor to provide its own glsl compiler. But this has nothing to do with Vulkan.
Second, how the fuck are you supposed to write some spirv bytecode directly
I don't quite understand this sentence (also, borderline rude), both hlsl and glsl can be converted to spir-v via glslang. It's just a different project. spirv-opt is the optimizer for already produced spir-v, and doesn't know or care how that spir-v was produced, as long as it's spec-conforming. For all you know it could come from msl, glsl, hlsl or some other shader language not yet invented. You could hand-write it if it made you happier.
It definitely looks like vulkan wasnt meant to be opengl improvement
Vulkan is meant to be another, totally different API. It's not a replacement for OpenGL, it depends on what you want to do. You're free to use DX11 if you don't like DX12, same here. However, Vulkan does better map to existing hardware which is why you get better performance, at the cost of more effort on your part.
Getting back to the original issue, I propose an option more like: --reduce-size-exhaustive-inlining.
Two reasons:
1) "reduce-size" because spirv-opt does other things besides reduce size (eg. specialization constant manipulation), so "do-everything" isn't quite apt.
2) "-exhaustive-inlining" because the SPIR-V WG has stated that they don't want exhaustive inlining to be done by default. Since some parties would like a mode that does exhaustive inlining (it can increase potential for downstream optimizations), I propose creating such an option where exhaustive inlining is explicit. When the "no-growth" inlining is available, we can create another option "--reduce-size-no-growth-inlining" which uses the "no-growth" inlining policy.
Is there an umbrella option to invoke all size-reducing transforms only, which would be a superset of a size-reducing policy for inlining?
For HLSL, it would also be good to have an inline policy that is the union of "size reducing" and "needed for validality". I imagine, generally, selecting multiple transforms means "union", so this should be somewhat natural to wrap up into an umbrella option.
I agree with the need for umbrella options with an inline policy that is a union of size reduction and validity. Of course, exhaustive does already cover validity, but other inlining policies might need additional inlining to be valid.
@redorav, the most i could find about glslang is that is is experimental, has tons of bugs, many glsl features are missing and so on. That really puts a weight on vulkan to fix this mess, i dont see how all that can be an improvement, or is vulkan supporting this open source attitude "alpha version forever" ?
@startas glslang is incomplete in some aspects (I wouldn't call it experimental at all though), but we're shipping an hlsl -> spir-v -> spirv-opt -> glsl relatively big game with it, using glslang, spirv-opt and SPIRVCross, all three of which are open source projects. Bear in mind using SPIRVCross adds an extra layer of complexity as opposed to using just SPIR-V and Vulkan. Using hlsl as source language is also more complex as support for it is newer than glsl. Plus, we're also using an established engine, not writing from scratch, so had to tailor the code to our needs. And we've still been able to do it. Granted, glslang needs time to mature and I can't say it's been trivial getting it to work, but along the way bug reporting and great support by the devs has made a big difference. I can say that the next project is going to be much easier.
Getting back to the original issue, I propose an option more like: --reduce-size-exhaustive-inlining.
May want to make "inline policy" an orthogonal set of switches, e.g.: --reduce-size --inline-exhaustive.
@startas glslang is multiple things, in more than one state. High-level summary:
May want to make "inline policy" an orthogonal set of switches, e.g.: --reduce-size --inline-exhaustive.
I thought about that but that felt like it defeated the purpose of a "do-everything" option.
I am now considering the option name --do-all-size-exhaustive. I like this because the "do-all" part seems to better suggest the idea of doing multiple, indeed all, related passes.
FYI @dnovillo
@dneto0 thanks. So, this is something that I've started looking at this week. My initial thoughts are to add a family of -Ox flags like other compilers do:
In addition, I'd like to create a configuration harness to allow users to express different transformation sequences from the outside. This is useful in many contexts (fuzzing and optimization space exploration among them).
-Oconfig=passes.cfg
Would something like this make sense in spirvopt?
The config harness idea sounds good.
The concern I have with the -Os flags is how to deal with the issue of exhaustive inlining. The SPIR WG has said they don't want exhaustive inlining to be done by "default". I am presuming by this that they would not approve of exhaustive inlining to be done in -Os. Anyone disagree?
Would the -Os flags replace the --do-all-size-exhaustive idea? If so, how would we express --do-all-size-exhaustive with the new regime?
On Wed, Aug 30, 2017 at 3:13 PM, greg-lunarg notifications@github.com wrote:
The config harness idea sounds good.
The concern I have with the -Os flags is how to deal with the issue of exhaustive inlining. The SPIR WG has said they don't want exhaustive inlining to be done by "default". I am presuming by this that they would not approve of exhaustive inlining to be done in -Os. Anyone disagree?
-Os should do minimal inlining. It's optimizing for size. Inlining goes to the other end of the spectrum.
Would the -Os flags replace the --do-all-size-exhaustive idea? If so, how would we express --do-all-size-exhaustive with the new regime?
I think they're semantically similar. In general, -Os means make the code as small as possible. Is that what '--do-all-size-exhaustive' would do?
I don't have data to back this up, but exhaustive inlining may or may not be optimal for code size, depending upon how a particular application organizes its SPIR-V code. There are many HLSL code bases which rely on the combination of exhaustive inlining and constant folding to produce reasonable code ( for example, passing boolean literals to functions to turn on or off various features, or passing constants into helper functions which are eventually folded). If an application follows the D3D pattern for SPIR-V shaders (one SPIR-V module per shader), then there is a good chance that exhaustive inlining will produce significant size benefits.
The fact that this is so content-dependent (and politicized) suggests separating exhaustive inlining from the rest of the concerns. A special switch which says "do exhaustive inlining first" seems like it would meet everybody's needs. The other flags would still be used to inform decisions like loop unrolling and if flattening.
opt --inline-exhaustive -Os // inline everything and then try to keep code small
opt --inline-exhaustive -Ox // inline everything and make things fast (unroll all the loops)
-Os should do minimal inlining. It's optimizing for size. Inlining goes to the other end of the spectrum.
Inlining is the only way right now that we do interprocedural analysis ie. constant propagation across function calls. So lack of inlining currently could prevent further optimization down the line and result in a larger result rather than smaller.
We are getting acceptable code sizes for our client's shaders with exhaustive inlining and unacceptable sizes with exhaustive inlining disabled.
Exhaustive inlining is the only inlining we have at the moment. An option that does not do inlining will be fairly useless to my client (and probably most everyone else). So for the moment, I would like to offer a "do-everything" option that does inlining, albeit exhaustive.
While I certainly believe there are real-life shaders that would do better with a more judicious inlining policy, fxc does exhaustive inlining so pretty much every HLSL shader that pre-dates Vulkan should be "fine" with exhaustive inlining.
My intention is to write a "no growth" inliner in the near future and see if we see as good or better results with it, but at the moment helping glslang create valid SPIR-V has higher priority.
On Wed, Aug 30, 2017 at 4:47 PM, greg-lunarg notifications@github.com wrote:
-Os should do minimal inlining. It's optimizing for size. Inlining goes to the other end of the spectrum.
Inlining is also the only way right now that we do interprocedural analysis ie. constant propagation across function calls. So lack of inlining currently could prevent further optimization down the line and result in a larger result rather than smaller.
Yeah, that's typically true. Inlining is the great enabler for IPO. But I wasn't familiar with the concept that extensive inlining would make the code smaller. I can see how this could happen though.
We are getting acceptable code sizes for our client's shaders with exhaustive inlining and unacceptable sizes with exhaustive inlining disabled.
Exhaustive inlining is the only inlining we have at the moment. An option that does not do inlining will be fairly useless to my client (and probably most everyone else). So for the moment, I would like to offer a "do-everything" option that does inlining, albeit exhaustive.
Sure, and -O would always do exhaustive inlining. I'm not familiar with the spirvopt inliner, so I'm not sure what "exhaustive inlining" means in this context. If we can prove that exhaustive inlining reduces code size, then by all means, let's introduce it in -Os.
My idea is for -O and -Os to simply be macro expanders for flags in spirvopt. So if you say '-Os --flag1 --flag2' it would expand to the list of options that we define for -Os and add --flag1 and --flag2 to the sequence.
While I certainly believe there are real-life shaders that would do better
with a more judicious inlining policy, fxc does exhaustive inlining so pretty much every HLSL shader that pre-dates Vulkan should be "fine" with exhaustive inlining.
My intention is to write a "no growth" inliner in the near future and see if we see as good or better results with it, but at the moment helping glslang create valid SPIR-V has higher priority.
Sounds good.
Diego.
On Wed, Aug 30, 2017 at 4:38 PM, jbarczak notifications@github.com wrote:
opt --inline-exhaustive -Os // inline everything and then try to keep code small opt --inline-exhaustive -Ox // inline everything and make things fast (unroll all the loops)
In my proposal this is exactly how it would work. The sequence of transformations would be left to right in the command line. First do --inline-exhaustive and then do the sequence defined by -Os or -Ox.
opt --inline-exhaustive -Os // inline everything and then try to keep code small opt --inline-exhaustive -Ox // inline everything and make things fast (unroll all the loops)
I continue to have a concern with this approach: I think it is a good idea to create a single option to "do everything", but if -Os is the answer and it doesn't do inlining, then it won't work very well and thus is not "doing everything".
I am worried that this will be suprising and unintuitive. If we are going to the trouble of consolidating size reduction passes under an option, then it seems to me we should go all the way. Our goal is to make these tools as easy to use as possible, but creating and advertising an option that supposedly does everything but then doesn't do the one thing that enables everything else and requires another option to precede it seems like it will be confusing and less helpful than it could be.
On Wed, Aug 30, 2017 at 5:59 PM, greg-lunarg notifications@github.com wrote:
opt --inline-exhaustive -Os // inline everything and then try to keep code small opt --inline-exhaustive -Ox // inline everything and make things fast (unroll all the loops)
I continue to have a concern with this approach: I think it is a good idea to create a single option to "do everything", but if -Os is the answer and it doesn't do inlining, then it won't work very well and thus is not "doing everything".
I think it's simply a matter of deciding to make -Os expand into the inliner flags if that's helpful in general. If it is, then by all means we should put it in.
I am worried that this will be suprising and unintuitive. If we are going to the trouble of consolidating size reduction passes under an option, then it seems to me we should go all the way. Our goal is to make these tools as easy to use as possible, but creating and advertising an option that supposedly does everything but then doesn't do the one thing that enables everything else and requires another option to precede it seems like it will be confusing and less helpful than it could be.
Agreed.
Just catching up with this thread.
Point of order:
My opinions:
I agree that spirv-opt is mysterious and inconvenient. So far it has just mimicked the design pattern of LLVM's "opt" tool: do exactly what I request, in the order I request it. That's an expert's tool. It's supposed to give very fine control to people who know the pieces very very well. It provides mechanism but no policy. But that's awful for all but a small handful of people. So I'll grant we need something much MUCH more friendly for most users.
However, "do everything" is ill-formed because there are many goals, including:
To reach a particular objective, you combine various pieces (the mechanism) to apply a particular recipe that matches the objective. A recipe is a selection, sequencing, and parameterization of individual transforms. In effect a recipe is policy.
Various stakeholders are very sensitive to recipes (policies) because they can make the code far worse for their platforms. This is especially true for default recipes.
Exhaustive inlining is definitely bad for some kinds of code. A lot depends on the application code and the compiler. For example the SPIR-V emitted by https://github.com/google/clspv already is pretty good about eliminating simple redundancies. The recipe that @greg-lunarg describes in the whitepaper is bad for the code coming out of clspv because many of those transforms end up as no-ops. The original code from clspv is pre-optimized, and the recipe in effect only does the exhaustive inlining part, which can blow up on you.
I think the way forward is:
At present, spirv-opt with no arguments simply emits the input SPIR-V unmodified. This is not particularly useful. While the command line interface is quite programmable and configurable, it is also not accessible. An engineer without any compiler expertise probably does not have the required knowledge to set this up, and is likely to copy some switches verbatim from a google search (the same applies to the programmatic interface in optimizer.hpp)
The interface is burdensome even for a knowledgeable user. In order to select the right configuration, the user needs to understand exactly what the available passes do, and how their results of one pass might interact with another. The user must think through the entire optimization pipeline in order to select the right set of switches, and may need to repeat this process every time new optimizations are introduced.
As with other compilers, there is going to be a universal set of things that a user would want done (for example, constant folding and DCE), and an obvious, correct order in which to do them (constant folding comes before DCE). It would be valuable to provide a single interface to enable all of the desirable transforms, with suitable default parameters, and run them in the right order, similar to the --do-everything switch in SPIRV-remap, or the -O switches in clang/gcc. This would make opt much more useful, and would also provide forward-compatibility for scripts and tools, without requiring manual intervention every time a new optimization comes online.