Bug 197151 – WHLSLPrepare.cpp always recompiles, even if nothing was changed

• Status: RESOLVED
• Resolution: FIXED
• Component: WebGPU

[Darin Adler]

WHLSLPrepare.cpp always recompiles, even if nothing was changed

[Darin Adler]

Created attachment 367920 [details]
Patch

[Darin Adler]

Created attachment 367921 [details]
Patch

[Darin Adler]

One small thing I don’t understand is why generate-xcfilelists did not remove the reference to WHLSLStandardLibrary.cpp even after I removed all other references to that filename from the source tree.

[Darin Adler]

Committed r244495: <https://trac.webkit.org/changeset/244495>

[Radar WebKit Bug Importer]

<rdar://problem/50083499>

[Shawn Roberts]

Changes in https://trac.webkit.org/changeset/244495 are causing build failures on Mac and iOS Release and Debug queues.

https://build.webkit.org/builders/Apple%20Mojave%20Release%20%28Build%29/builds/5103/steps/compile-webkit/logs/stdio

perl JavaScriptCorePrivateHeaders/xxd.pl XMLViewer_js ./XMLViewer.min.js XMLViewerJS.h
Can't modify anonymous list ([]) in scalar assignment at JavaScriptCorePrivateHeaders/xxd.pl line 3, near "};"
Execution of JavaScriptCorePrivateHeaders/xxd.pl aborted due to compilation errors.
make: *** [XMLViewerJS.h] Error 255
perl JavaScriptCorePrivateHeaders/xxd.pl CommandLineAPIModuleSource_js ./CommandLineAPIModuleSource.min.js CommandLineAPIModuleSource.h
Can't modify anonymous list ([]) in scalar assignment at JavaScriptCorePrivateHeaders/xxd.pl line 3, near "};"
Execution of JavaScriptCorePrivateHeaders/xxd.pl aborted due to compilation errors.
make: *** [CommandLineAPIModuleSource.h] Error 255

[WebKit Commit Bot]

Re-opened since this is blocked by bug 197159

[Darin Adler]

I have no idea how this change caused those problems. Does anyone know?

[Darin Adler]

Myles, at some point when you are free after doing higher priority work, would you be willing to help me with this?

[Darin Adler]

I am still getting this error in xxd.pl in my build here on my Mac even though my source tree does not have the changes from this patch; no time to investigate and fix it right now. Are we sure about cause/effect here?

[Darin Adler]

Oh wait, maybe I do have the changes; I’ll be able to debug this locally now. No worries, no rush.

[Darin Adler]

I can no longer reproduce this error in xxd.pl so I don’t know how to make progress on this. Can someone help me reproduce the problem? I’d love to fix this makefile mistake because it’s really annoying to do extra compiling every time I build!

[Darin Adler]

This bug is really annoying me — I’d love to land the change some day, but I need help reproducing the build failure that meant Shawn had to roll it out last time. Especially mysterious given the changes don’t seem to be ones that could affect those other invocations of xxd.pl

[Keith Rollin]

(In reply to Darin Adler from comment #3)
> One small thing I don’t understand is why generate-xcfilelists did not
> remove the reference to WHLSLStandardLibrary.cpp even after I removed all
> other references to that filename from the source tree.

xcfilelists need to represent all platforms and configurations, and must be a super-set of the files used for all those build combinations. However, when updating the xcfilelists during build time, we can only update the xcfilelists with respect to that current build. This means that the xcfilelist contents generated during the build might be a subset of what's required for all build combinations. Since it's a subset, there might be files in the resulting xcfilelist that we could characterize as "left over". And there's no way to know if those files are obsolete and can be removed, or if they are needed by a different build combination. Therefore, during the incremental xcfilelist updates, we can't safely delete any files.

There's some discussion of this at <https://confluence.sd.apple.com/pages/viewpage.action?pageId=783873680> under Known Issues.

[Keith Rollin]

Comment on attachment 367921 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=367921&action=review

> Source/WebCore/DerivedSources.make:1639
> +	$(PERL) $(JavaScriptCore_SCRIPTS_DIR)/xxd.pl WHLSLStandardLibrary $(WebCore)/Modules/webgpu/WHLSL/WHLSLStandardLibrary.txt $<

Is this an equivalent replacement? You're replacing "WHLSLStandardLibrary.h" with "$<". But $< is the first pre-requisite, which is the xxd.pl tool. I think you want $@ instead of $<.

[Darin Adler]

Comment on attachment 367921 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=367921&action=review

>> Source/WebCore/DerivedSources.make:1639
>> +	$(PERL) $(JavaScriptCore_SCRIPTS_DIR)/xxd.pl WHLSLStandardLibrary $(WebCore)/Modules/webgpu/WHLSL/WHLSLStandardLibrary.txt $<
> 
> Is this an equivalent replacement? You're replacing "WHLSLStandardLibrary.h" with "$<". But $< is the first pre-requisite, which is the xxd.pl tool. I think you want $@ instead of $<.

Yes, I want $@, that’s the bug. I was overwriting the tool!

[Darin Adler]

Thank you Keith!

[Darin Adler]

Committed r245233: <https://trac.webkit.org/changeset/245233>