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

-mfmt=bin/c/num to emit binary as it is, numbers or C init list #218

Closed
wants to merge 1 commit into from

Conversation

Qining
Copy link
Contributor

@Qining Qining commented May 15, 2016

Issue: #214

Add command line option: -masm=c-init-list to emit SPIRV binary code as
a C-style uint32_t array initializer list in the output file.

-masm=c-init-list should only be specified when the compilation output
is SPIRV binary code. When used with disassembly mode (-S) and
preprocessing-only mode (-E), it will result in error and fail the
compilation. Note that dumping dependency info as compilation output (-M
or -MM) also implies preprocessing-only (-E) and will lead to error.
But dumpping dependency info to extra .d file (-MD or -MMD) won't cause
error as the compilation output is still SPIRV binary code.

@@ -0,0 +1,127 @@
# Copyright 2015 The Shaderc Authors. All rights reserved.
Copy link
Contributor

Choose a reason for hiding this comment

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

This is year 2016. :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

@Qining
Copy link
Contributor Author

Qining commented May 16, 2016

|c-init-list |Output SPIRV binary code as a text file containing C-style +
initializer list. This option is valid when the compilation +
output is SPIRV binary code. It is not valid to use when the +
output is not SPIV binary code, like when `-S`, `-E`, `-M`, +
Copy link

Choose a reason for hiding this comment

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

typo: SPIR-V

@dneto0
Copy link
Collaborator

dneto0 commented May 16, 2016

done my review.
High level item: I think -masm should only take effect with -S

@dneto0
Copy link
Collaborator

dneto0 commented May 16, 2016

Discussed offline.
To address my complaint, we'll change the name of the option to -mfmt
and have options "c" for what is now c-init-list, "bin" being the current binary output form (the default), and "num" for a list of comma-separated numbers.

@Qining Qining force-pushed the compile-to-uint32-array branch 2 times, most recently from d065f90 to 475fd3b Compare May 17, 2016 13:53
@Qining
Copy link
Contributor Author

Qining commented May 17, 2016

@dekimir @dneto0 @AWoloszyn @antiagainst PTAL.

@Qining Qining changed the title -masm=c-init-list to emit binary as C init list -mfmt=bin/c/num to emit binary as it is, numbers or C init list May 17, 2016
|c |Output SPIR-V binary code as a text file containing C-style +
initializer list. +
This is just wrapping the output of `num` option with curly
brakets.
Copy link
Collaborator

Choose a reason for hiding this comment

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

"brackets".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in d7942c1

==== `-mfmt=<format>`

Select output format for compilation output in SPIR-V binary code form.
Supported options are listed in <<binary-output-format-selection,binary output
Copy link
Contributor

Choose a reason for hiding this comment

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

in the ... table.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in d7942c1

@dneto0
Copy link
Collaborator

dneto0 commented May 17, 2016

Done review. Plus advice to expose binary emission format as public inner class to the FileCompiler, and have a single set-bin-output-format method on FileCompiler.

@dneto0
Copy link
Collaborator

dneto0 commented May 18, 2016

+2

Add command line option: `-mfmt=<fmt>` to select the emission format for
SPIR-V binary output. Available formats are `bin` (default): a sequence of binary
32-bitwords in host native endianness; `num` a list of comma-separated
hex numbers in text form; `c` a C-style initializer list of hex numbers.

`-mfmt=<fmt>` should only be specified when the compilation output
is SPIRV binary code. When used with disassembly mode (-S) and
preprocessing-only mode (-E), it will result in error and fail the
compilation. Note that dumping dependency info as compilation output (-M
or -MM) also implies preprocessing-only (-E) and will lead to error.
But dumpping dependency info to extra .d file (-MD or -MMD) won't cause
error as the compilation output is still SPIRV binary code.
@Qining
Copy link
Contributor Author

Qining commented May 18, 2016

Squashed and merged

@Qining Qining closed this May 18, 2016
Kangz pushed a commit to Kangz/shaderc that referenced this pull request Sep 3, 2019
Previously, the renumbering loop would sometimes iterate in the wrong order. To fix this, instead use the binding info already correctly extracted by `ExtractSpirvInfo`.

Fixes ComputeCopyStorageBufferTests.BasicTest/D3D12.
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.

None yet

5 participants