[sycl-post-link] Add property with the default values of specialization constants #3666

vmaksimo · 2021-04-30T16:43:13Z

This patch introduces a new property which contains all the defaults for spec
constants.
It is done by setting of metadata with the default values:

; Compilation line:
; sycl-post-link -spec-const=default -S llvm/test/tools/sycl-post-link/spec-constants/SYCL-2020.ll -o property.table
!5 = !{i32 42}
!6 = !{%struct.ComposConst { i32 1, double 2.000000e+00, %struct.myConst { i32 13, float 0x4020666660000000 } }}

Property set looks this way:

[SYCL/specialization constants default values]
all=2|gAAAAAAAAAgKAAAA

…on constants This patch introduces a new property which contains all the defaults for spec constants. It is done by setting of metadata with the default values: ``` ; Compilation line: ; sycl-post-link -spec-const=default -S llvm/test/tools/sycl-post-link/spec-constants/SYCL-2020.ll -o property.table !5 = !{i32 42} !6 = !{%struct.ComposConst { i32 1, double 2.000000e+00, %struct.myConst { i32 13, float 0x4020666660000000 } }} ``` Property set looks this way: ``` [SYCL/specialization constants default values] all=2|gAAAAAAAAAgKAAAA ```

AlexeySachkov

Partial review

llvm/tools/sycl-post-link/SpecConstants.cpp

AlexeySachkov · 2021-04-30T17:14:57Z

llvm/tools/sycl-post-link/SpecConstants.cpp

+    Offset += Ty->getPrimitiveSizeInBits() / 8 +
+              (Ty->getPrimitiveSizeInBits() % 8 != 0);


Since you are appending to a vector and not inserting into a pre-allocated vector, you probably don't need this argument anymore

Offset argument is actually used for calculations in StructTy - it helps to fill the buffer with the correct alignment:
https://github.com/intel/llvm/pull/3666/files#diff-f9a582625f70ea58ef88b7713c591e8187629cca507b424557a3e4664e23848fR308-R312

llvm/tools/sycl-post-link/SpecConstants.cpp

AlexeySachkov

And the rest of the review

llvm/tools/sycl-post-link/SpecConstants.cpp

llvm/tools/sycl-post-link/sycl-post-link.cpp

llvm/include/llvm/Support/PropertySetIO.h

kbobrovs · 2021-05-05T22:17:38Z

llvm/tools/sycl-post-link/SpecConstants.cpp

+/// Recursively iterates over a composite type in order to collect information
+/// about default values of its scalar elements.
+void collectCompositeElementsDefaultValuesRecursive(
+    const Module &M, Constant *C, unsigned &Offset,


NIT: processing of composite spec constants is similar in this function, emitSpecConstantRecursiveImpl and generateSpecConstDefaultValueMetadata. Could be factored out using e.g. the visitor pattern. At least a TODO for now would be good.

kbobrovs · 2021-05-05T22:27:09Z

llvm/tools/sycl-post-link/SpecConstants.cpp

+          // default value of corresponding specialization constant.
+          DefaultValue = Initializer->getAggregateElement(0u);
+        }
+      }


what GV == 0 means? please add a comment or handle

Probably we should just use cast instead of dyn_cast and always expect to see a value there.

We can meet nullptr instead of value here - this check is still needed. I left a comment above the if-.

llvm/tools/sycl-post-link/SpecConstants.cpp

kbobrovs · 2021-05-08T07:03:07Z

llvm/tools/sycl-post-link/SpecConstants.cpp

+/// Recursively iterates over a composite type in order to collect information
+/// about default values of its scalar elements.
+/// TODO: processing of composite spec constants here is similar to
+/// collectCompositeElementsInfoRecursive. Possible place for improvement -


Is it also similar to emitSpecConstantRecursiveImpl and generateSpecConstDefaultValueMetadata?

Not really, it doesn't look similar. generateSpecConstDefaultValueMetadata is not related to recursive methods, and emitSpecConstantRecursiveImpl is too different from this function.

kbobrovs

added few comments

AlexeySachkov · 2021-05-11T08:55:21Z

llvm/tools/sycl-post-link/SpecConstants.cpp

+          // default value of corresponding specialization constant.
+          DefaultValue = Initializer->getAggregateElement(0u);
+        }
+      }


Probably we should just use cast instead of dyn_cast and always expect to see a value there.

llvm/tools/sycl-post-link/SpecConstants.cpp

alexbatashev

RT changes look good

bader · 2021-05-13T12:16:01Z

@kbobrovs, ping.

…binaries DefaultValues.DefaultValuesAreSet and DefaultValues.DefaultValuesAreOverriden tests only support JIT which is not the case for the default values. Once AOT binaries support is added, the tests can be enabled back.

AlexeySachkov · 2021-05-24T14:47:21Z

sycl/source/detail/device_image_impl.hpp

          MSpecConstSymMap[std::string{SCName}].push_back(
              SpecConstDescT{/*ID*/ It[0], /*CompositeOffset*/ It[1],
-                             /*Size*/ It[2], BlobOffset, HasDefaultValues});
+                             /*Size*/ It[2], BlobOffset});


I'm a bit concerned that this change is being committed in scope of this PR. The PR title says that this is a modification of sycl-post-link tool and all 13 commits are very likely to be squashed into a single one during the merge - we will end up with a commit to sycl-post-link, which modifies DPC++ RT.

Moreover, it is not clear to me why this change is needed at all. From my point of view it is better if we submit it in a separate PR

Tagging @dm-vodopyanov here

This change is related to https://github.com/intel/llvm/pull/3667/files#r631154823. I'll create a separate PR to make it clear.

llvm/include/llvm/Support/PropertySetIO.h

kbobrovs · 2021-05-24T19:16:03Z

llvm/include/llvm/Support/PropertySetIO.h

      PropSet.insert(std::make_pair(Prop.first, PropertyValue(Prop.second)));
  }

+  template <typename T>


Please add a comment when this function should be used instead of void add(StringRef Category, const std::map<StringRef, T> &Props) above. I.e. when the order of properties matters.

Suggest to make add templated on the map type to have single implementation for both std::map<StringRef, T> and std::map<StringRef, T>

Remove backward compatibility according to discussion: https://github.com/intel/llvm/pull/3667/files#r631154823i This patch is expected to be merged together or before #3666

Moving to intel#3813

Bumps [urllib3](https://github.com/urllib3/urllib3) from 2.5.0 to 2.6.0. <details> <summary>Release notes</summary> <p><em>Sourced from <a href="https://github.com/urllib3/urllib3/releases">urllib3's releases</a>.</em></p> <blockquote> <h2>2.6.0</h2> <h2>🚀 urllib3 is fundraising for HTTP/2 support</h2> <p><a href="https://sethmlarson.dev/urllib3-is-fundraising-for-http2-support">urllib3 is raising ~$40,000 USD</a> to release HTTP/2 support and ensure long-term sustainable maintenance of the project after a sharp decline in financial support. If your company or organization uses Python and would benefit from HTTP/2 support in Requests, pip, cloud SDKs, and thousands of other projects <a href="https://opencollective.com/urllib3">please consider contributing financially</a> to ensure HTTP/2 support is developed sustainably and maintained for the long-haul.</p> <p>Thank you for your support.</p> <h2>Security</h2> <ul> <li>Fixed a security issue where streaming API could improperly handle highly compressed HTTP content ("decompression bombs") leading to excessive resource consumption even when a small amount of data was requested. Reading small chunks of compressed data is safer and much more efficient now. (CVE-2025-66471 reported by <a href="https://github.com/Cycloctane"><code>@Cycloctane</code></a>, 8.9 High, GHSA-2xpw-w6gg-jr37)</li> <li>Fixed a security issue where an attacker could compose an HTTP response with virtually unlimited links in the <code>Content-Encoding</code> header, potentially leading to a denial of service (DoS) attack by exhausting system resources during decoding. The number of allowed chained encodings is now limited to 5. (CVE-2025-66418 reported by <a href="https://github.com/illia-v"><code>@illia-v</code></a>, 8.9 High, GHSA-gm62-xv2j-4w53)</li> </ul> <blockquote> <p>[!IMPORTANT]</p> <ul> <li>If urllib3 is not installed with the optional <code>urllib3[brotli]</code> extra, but your environment contains a Brotli/brotlicffi/brotlipy package anyway, make sure to upgrade it to at least Brotli 1.2.0 or brotlicffi 1.2.0.0 to benefit from the security fixes and avoid warnings. Prefer using <code>urllib3[brotli]</code> to install a compatible Brotli package automatically.</li> <li>If you use custom decompressors, please make sure to update them to respect the changed API of <code>urllib3.response.ContentDecoder</code>.</li> </ul> </blockquote> <h2>Features</h2> <ul> <li>Enabled retrieval, deletion, and membership testing in <code>HTTPHeaderDict</code> using bytes keys. (<a href="https://redirect.github.com/urllib3/urllib3/issues/3653">#3653</a>)</li> <li>Added host and port information to string representations of <code>HTTPConnection</code>. (<a href="https://redirect.github.com/urllib3/urllib3/issues/3666">#3666</a>)</li> <li>Added support for Python 3.14 free-threading builds explicitly. (<a href="https://redirect.github.com/urllib3/urllib3/issues/3696">#3696</a>)</li> </ul> <h2>Removals</h2> <ul> <li>Removed the <code>HTTPResponse.getheaders()</code> method in favor of <code>HTTPResponse.headers</code>. Removed the <code>HTTPResponse.getheader(name, default)</code> method in favor of <code>HTTPResponse.headers.get(name, default)</code>. (<a href="https://redirect.github.com/urllib3/urllib3/issues/3622">#3622</a>)</li> </ul> <h2>Bugfixes</h2> <ul> <li>Fixed redirect handling in <code>urllib3.PoolManager</code> when an integer is passed for the retries parameter. (<a href="https://redirect.github.com/urllib3/urllib3/issues/3649">#3649</a>)</li> <li>Fixed <code>HTTPConnectionPool</code> when used in Emscripten with no explicit port. (<a href="https://redirect.github.com/urllib3/urllib3/issues/3664">#3664</a>)</li> <li>Fixed handling of <code>SSLKEYLOGFILE</code> with expandable variables. (<a href="https://redirect.github.com/urllib3/urllib3/issues/3700">#3700</a>)</li> </ul> <h2>Misc</h2> <ul> <li>Changed the <code>zstd</code> extra to install <code>backports.zstd</code> instead of <code>zstandard</code> on Python 3.13 and before. (<a href="https://redirect.github.com/urllib3/urllib3/issues/3693">#3693</a>)</li> <li>Improved the performance of content decoding by optimizing <code>BytesQueueBuffer</code> class. (<a href="https://redirect.github.com/urllib3/urllib3/issues/3710">#3710</a>)</li> <li>Allowed building the urllib3 package with newer setuptools-scm v9.x. (<a href="https://redirect.github.com/urllib3/urllib3/issues/3652">#3652</a>)</li> <li>Ensured successful urllib3 builds by setting Hatchling requirement to ≥ 1.27.0. (<a href="https://redirect.github.com/urllib3/urllib3/issues/3638">#3638</a>)</li> </ul> </blockquote> </details> <details> <summary>Changelog</summary> <p><em>Sourced from <a href="https://github.com/urllib3/urllib3/blob/main/CHANGES.rst">urllib3's changelog</a>.</em></p> <blockquote> <h1>2.6.0 (2025-12-05)</h1> <h2>Security</h2> <ul> <li>Fixed a security issue where streaming API could improperly handle highly compressed HTTP content ("decompression bombs") leading to excessive resource consumption even when a small amount of data was requested. Reading small chunks of compressed data is safer and much more efficient now. (<code>GHSA-2xpw-w6gg-jr37 <https://github.com/urllib3/urllib3/security/advisories/GHSA-2xpw-w6gg-jr37></code>__)</li> <li>Fixed a security issue where an attacker could compose an HTTP response with virtually unlimited links in the <code>Content-Encoding</code> header, potentially leading to a denial of service (DoS) attack by exhausting system resources during decoding. The number of allowed chained encodings is now limited to 5. (<code>GHSA-gm62-xv2j-4w53 <https://github.com/urllib3/urllib3/security/advisories/GHSA-gm62-xv2j-4w53></code>__)</li> </ul> <p>.. caution::</p> <ul> <li> <p>If urllib3 is not installed with the optional <code>urllib3[brotli]</code> extra, but your environment contains a Brotli/brotlicffi/brotlipy package anyway, make sure to upgrade it to at least Brotli 1.2.0 or brotlicffi 1.2.0.0 to benefit from the security fixes and avoid warnings. Prefer using <code>urllib3[brotli]</code> to install a compatible Brotli package automatically.</p> </li> <li> <p>If you use custom decompressors, please make sure to update them to respect the changed API of <code>urllib3.response.ContentDecoder</code>.</p> </li> </ul> <h2>Features</h2> <ul> <li>Enabled retrieval, deletion, and membership testing in <code>HTTPHeaderDict</code> using bytes keys. (<code>[#3653](urllib3/urllib3#3653) <https://github.com/urllib3/urllib3/issues/3653></code>__)</li> <li>Added host and port information to string representations of <code>HTTPConnection</code>. (<code>[#3666](urllib3/urllib3#3666) <https://github.com/urllib3/urllib3/issues/3666></code>__)</li> <li>Added support for Python 3.14 free-threading builds explicitly. (<code>[#3696](urllib3/urllib3#3696) <https://github.com/urllib3/urllib3/issues/3696></code>__)</li> </ul> <h2>Removals</h2> <ul> <li>Removed the <code>HTTPResponse.getheaders()</code> method in favor of <code>HTTPResponse.headers</code>. Removed the <code>HTTPResponse.getheader(name, default)</code> method in favor of <code>HTTPResponse.headers.get(name, default)</code>. (<code>[#3622](urllib3/urllib3#3622) <https://github.com/urllib3/urllib3/issues/3622></code>__)</li> </ul> <h2>Bugfixes</h2> <ul> <li>Fixed redirect handling in <code>urllib3.PoolManager</code> when an integer is passed for the retries parameter. (<code>[#3649](urllib3/urllib3#3649) <https://github.com/urllib3/urllib3/issues/3649></code>__)</li> <li>Fixed <code>HTTPConnectionPool</code> when used in Emscripten with no explicit port. (<code>[#3664](urllib3/urllib3#3664) <https://github.com/urllib3/urllib3/issues/3664></code>__)</li> <li>Fixed handling of <code>SSLKEYLOGFILE</code> with expandable variables. (<code>[#3700](urllib3/urllib3#3700) <https://github.com/urllib3/urllib3/issues/3700></code>__)</li> </ul>  </blockquote> <p>... (truncated)</p> </details> <details> <summary>Commits</summary> <ul> <li><a href="https://github.com/urllib3/urllib3/commit/720f484b605f18887a48eef448d0084e2b76902d"><code>720f484</code></a> Release 2.6.0</li> <li><a href="https://github.com/urllib3/urllib3/commit/24d7b67eac89f94e11003424bcf0d8f7b72222a8"><code>24d7b67</code></a> Merge commit from fork</li> <li><a href="https://github.com/urllib3/urllib3/commit/c19571de34c47de3a766541b041637ba5f716ed7"><code>c19571d</code></a> Merge commit from fork</li> <li><a href="https://github.com/urllib3/urllib3/commit/816fcf04528bc0f89672e13398eb813dcc892490"><code>816fcf0</code></a> Bump actions/setup-python from 6.0.0 to 6.1.0 (<a href="https://redirect.github.com/urllib3/urllib3/issues/3725">#3725</a>)</li> <li><a href="https://github.com/urllib3/urllib3/commit/18af0a10efc4c99dd028f7ad5a461470b9a8b0fd"><code>18af0a1</code></a> Improve speed of <code>BytesQueueBuffer.get()</code> by using memoryview (<a href="https://redirect.github.com/urllib3/urllib3/issues/3711">#3711</a>)</li> <li><a href="https://github.com/urllib3/urllib3/commit/1f6abac3e6d426c3939b8a17cf4afa099e691ab2"><code>1f6abac</code></a> Bump versions of pre-commit hooks (<a href="https://redirect.github.com/urllib3/urllib3/issues/3716">#3716</a>)</li> <li><a href="https://github.com/urllib3/urllib3/commit/1c8fbf787b8e6ed151842c5d6874c9d5bdbf1d0b"><code>1c8fbf7</code></a> Bump actions/checkout from 5.0.0 to 6.0.0 (<a href="https://redirect.github.com/urllib3/urllib3/issues/3722">#3722</a>)</li> <li><a href="https://github.com/urllib3/urllib3/commit/7784b9eee95b7c90802c02b111e98df70259ae4f"><code>7784b9e</code></a> Add Python 3.15 to CI (<a href="https://redirect.github.com/urllib3/urllib3/issues/3717">#3717</a>)</li> <li><a href="https://github.com/urllib3/urllib3/commit/0241c9e7286d3008e3cce18effc13b40dc633385"><code>0241c9e</code></a> Updated docs to reflect change in optional zstd dependency from <code>zstandard</code> t...</li> <li><a href="https://github.com/urllib3/urllib3/commit/7afcabb6489d9a8ea95a40e5afcb46463af17351"><code>7afcabb</code></a> Expand environment variable of SSLKEYLOGFILE (<a href="https://redirect.github.com/urllib3/urllib3/issues/3705">#3705</a>)</li> <li>Additional commits viewable in <a href="https://github.com/urllib3/urllib3/compare/2.5.0...2.6.0">compare view</a></li> </ul> </details> <br /> [![Dependabot compatibility score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=urllib3&package-manager=pip&previous-version=2.5.0&new-version=2.6.0)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) --- <details> <summary>Dependabot commands and options</summary> <br /> You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot merge` will merge this PR after your CI passes on it - `@dependabot squash and merge` will squash and merge this PR after your CI passes on it - `@dependabot cancel merge` will cancel a previously requested merge and block automerging - `@dependabot reopen` will reopen this PR if it is closed - `@dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually - `@dependabot show <dependency name> ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself) You can disable automated security fix PRs for this repo from the [Security Alerts page](https://github.com/intel/llvm/network/alerts). </details> Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

vmaksimo added 2 commits April 30, 2021 19:40

Support floats and composite types

45db4f1

vmaksimo requested a review from AlexeySachkov April 30, 2021 16:43

AlexeySachkov reviewed Apr 30, 2021

View reviewed changes

vmaksimo added 4 commits May 5, 2021 18:18

Apply suggestions from code review

eb594cf

Minor line changes

c632c64

Merge branch 'sycl' into spec_const_property_pub

41dc658

Do no generate metadata if default value ptr is null

4ca0b76

vmaksimo marked this pull request as ready for review May 5, 2021 16:48

vmaksimo requested review from a team, kbobrovs, mlychkov and smaslov-intel as code owners May 5, 2021 16:48

kbobrovs reviewed May 5, 2021

View reviewed changes

Minor changes after CR

d6fa1f9

vmaksimo requested review from AlexeySachkov and kbobrovs May 7, 2021 17:06

kbobrovs reviewed May 8, 2021

View reviewed changes

AlexeySachkov reviewed May 11, 2021

View reviewed changes

Apply comments

08de0d4

AlexeySachkov previously approved these changes May 11, 2021

View reviewed changes

remove changes from SYCL header pi.h

6d25843

vmaksimo dismissed AlexeySachkov’s stale review via 6d25843 May 11, 2021 10:55

alexbatashev previously approved these changes May 11, 2021

View reviewed changes

AlexeySachkov previously approved these changes May 11, 2021

View reviewed changes

AlexeySachkov removed the request for review from smaslov-intel May 11, 2021 16:57

bader requested a review from kbobrovs May 11, 2021 17:01

alexbatashev mentioned this pull request May 12, 2021

[SYCL] Add DPC++ RT support for SYCL2020 spec constants' default values #3667

Merged

kbobrovs previously approved these changes May 17, 2021

View reviewed changes

Merge branch 'sycl' into spec_const_property_defaults

a5cbe74

vmaksimo dismissed stale reviews from kbobrovs, AlexeySachkov, and alexbatashev via a5cbe74 May 18, 2021 09:11

AlexeySachkov previously approved these changes May 18, 2021

View reviewed changes

Merge remote-tracking branch 'origin/sycl' into pr_3666

19c8857

vmaksimo dismissed AlexeySachkov’s stale review via 05ead09 May 21, 2021 12:04

vmaksimo added 2 commits May 24, 2021 14:32

Fix test failure for non-native spec consts

774051b

vmaksimo force-pushed the spec_const_property_defaults branch from 57f8fbd to 02293fb Compare May 24, 2021 11:32

AlexeySachkov reviewed May 24, 2021

View reviewed changes

kbobrovs reviewed May 24, 2021

View reviewed changes

vmaksimo mentioned this pull request May 25, 2021

[SYCL] Avoid HasDefaultValues to be set as IsSet field of SpecConstDescT #3812

Closed

Minor improvement

c4e538a

vmaksimo mentioned this pull request May 25, 2021

[SYCL] Avoid HasDefaultValues to be set as IsSet field of SpecConstDescT #3813

Merged

Remove RT changes from this PR

9b0305b

Moving to intel#3813

kbobrovs previously approved these changes May 25, 2021

View reviewed changes

Fix typo

c3c7989

vmaksimo dismissed kbobrovs’s stale review via c3c7989 May 26, 2021 09:15

Merge remote-tracking branch 'origin/sycl' into pr_3666

df0d057

vmaksimo requested a review from kbobrovs May 26, 2021 16:50

AlexeySachkov approved these changes May 27, 2021

View reviewed changes

alexbatashev approved these changes May 27, 2021

View reviewed changes

romanovvlad merged commit 7d5ee05 into intel:sycl May 27, 2021

		Offset += Ty->getPrimitiveSizeInBits() / 8 +
		(Ty->getPrimitiveSizeInBits() % 8 != 0);

[sycl-post-link] Add property with the default values of specialization constants #3666

[sycl-post-link] Add property with the default values of specialization constants #3666

Uh oh!

Conversation

vmaksimo commented Apr 30, 2021

Uh oh!

AlexeySachkov left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

AlexeySachkov left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kbobrovs left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

alexbatashev left a comment

Choose a reason for hiding this comment

Uh oh!

bader commented May 13, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants