Hello Hans-Kristian,
On 1/29/20 9:31 AM, Hans-Kristian Arntzen wrote:
On 1/29/20 3:09 PM, Rémi Bernon wrote:
On 1/29/20 2:54 PM, Andrew Eikum wrote:
On Wed, Jan 29, 2020 at 12:51:25PM +0100, Hans-Kristian Arntzen wrote:
Hans-Kristian Arntzen (41):
16 files changed, 6544 insertions(+), 724 deletions(-)
I can't speak to the content of the series, but this is a huuuuuge amount of code to dump all at once. I'd suggest sending series of about 4-5 patches at a time.
Andrew
Hi,
I wanted to have a look and the patches don't apply cleanly either (it fails at #02, but later as well).
There seems to have been some commits in-between master and the patchset, which caused issue when I tried to apply them to clean master. Doesn't seem like there is much of a mood for getting another 41 commits on the ML, so just uploading a tarball instead if that's okay. Alternatively, a branch is here: https://github.com/HansKristian-Work/vkd3d/tree/dxil-review.
Any update on this?
I can't speak for Henri, but I expect that it would be preferred to resend the rebased patches to the mailing list, just in smaller batches (5 at a time, generally).
In response to your reply to Andrew Eikum, I suspect that submitting tests for DXIL before trying to submit an implementation would certainly be welcome.
Also I tried to build dxil-spirv on Debian, and it misses some LLVM include paths in the CMakeLists.txt. Fixing that is easy, but then it also requires another include fix to build with llvm-10. Then I tried several other version and LLVM 8, 9 and 10 seems to build fine, but it starts to fail compiling with earlier versions of LLVM.
In my experience, building a project using LLVM C++ API makes it very brittle, as they tend to change their API quite often. The projects fall behind very quickly as they cannot keep up with LLVM development speed, and it becomes very hard to build.
I'm not sure what's the target but I believe that since the LLVM IR format is advertised as stable since 3.7 -and DXIL format probably even more- depending on some unstable API to parse it goes a bit against it.
A better solution to this, IMHO, would be to have a custom parser for DXIL. Although it's a bit more work, the project would be more self contained and much more portable.
Yes, that's the ideal solution. I am aware of this problem, but I had hoped to defer this. Writing a good BC parser from scratch is likely another several man-months of work on top unless someone have made useful progress on that already.
Again, I can't speak for the vkd3d maintainer, but I suspect that they are not going to be particularly happy about separating the DXIL parser into a separate library, having to review C++ code, or the method of dependency management that you have chosen (namely, building third-party libraries not as dynamic, external dependencies; see [1] for further rationale).
[1] https://wiki.debian.org/UpstreamGuide
Cheers, Hans-Kristian
Cheers,