Skip to content

[Adreno] Enable static texture planning for constant data - #11357

Closed
elvin-n wants to merge 4 commits into
apache:mainfrom
Deelvin:scout/adreno_static_textures
Closed

[Adreno] Enable static texture planning for constant data#11357
elvin-n wants to merge 4 commits into
apache:mainfrom
Deelvin:scout/adreno_static_textures

Conversation

@elvin-n

Copy link
Copy Markdown
Contributor

Previous PR11161 added topi compute and schedules which manages textures in dynamic mode through explicit call of cache_read in adreno primitive schedules. This PR adds top-down approach of memory annotation plus adding support in dependent parts.

  1. Passes of analyzing memory scope for expressions in the relay graph and modifies relay graph by adding/modifying VirutalDevice with required memory scope
  2. Add support of memory scope into graph memory planner
  3. Support of static planned textures in json and opencl runtime
  4. Modification of Relay->TIR settling taking into account relay expr virtual devices for variables

@elvin-n
elvin-nforce-pushed the scout/adreno_static_textures branch 3 times, most recently from 35aec2e to 467d321CompareMay 24, 2022 12:42
@elvin-n
elvin-n marked this pull request as ready for review May 24, 2022 12:58
@elvin-n

Copy link
Copy Markdown
ContributorAuthor

@csullivan@mbs-octoml PR is ready for review

@mbs-octomlmbs-octoml left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks, this is quite the heroic piece of work.

Can I suggest splitting this into smaller chunks?

  • Plumbing mem scope through TE (and perhaps handle the mem-scope-per-output issue)
  • Mem scope in graph executor mem planner, codegen & runtime.
  • 1D & 2D mem pools in graph executor runtime
  • Schedules
  • New annotator pass, which perhaps should also be made target-specific since the rules for deriving scope from shape seem pretty target specific. Ie it should be moved to a target specific dir, and the pass can be the identity if the current target does not match. Unfortunately there is no generic mechanism for registering a target-specific pass that will work for this.

I'm not sure how to reconcile the fairly generic multi-dimensional buffer support we now have with the hard 1d vs 2d distinction you're adding in graph executor. Is there a discussion about that someplace I've missed?

I can't quite tell if your StorageInfo pass supports tuples & tuple projection. That whole pass needs commenting. But in any case worth mentioning you've eschewed the collect-and-solve constraint approach used by plan devices in favor of an eager back propagation of scopes from argument to constants. Fair enough, but please document the subset of relay this supports.

Comment threadsrc/driver/driver_api.cc Outdated
primitive_supports_texture_ = false;
Visit(call->op);
if (primitive_supports_texture_) {
if (call->checked_type().as<TensorTypeNode>()) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

See tvm::relay::FlattenTupleType

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

this is done by intend so far - if we use FlattenTupleType, we would analyze each tensor individually that is wrong for this case. Currently I have an impression that if we have tuple been planned in static memory planner, the memory scope should be unique for all tensor contained in tuple

Comment threadsrc/relay/transforms/annotate_texture_storage.cc
Comment threadinclude/tvm/te/operation.h Outdated
@elvin-n
elvin-nforce-pushed the scout/adreno_static_textures branch from f13a402 to 23e5d7fCompareJune 20, 2022 19:04
Co-authored-by: Chris Sullivan <csullivan@octoml.ai>
@elvin-n
elvin-nforce-pushed the scout/adreno_static_textures branch from 23e5d7f to b73066bCompareJune 20, 2022 19:08
@elvin-n
elvin-nforce-pushed the scout/adreno_static_textures branch from b73066b to 02bc11cCompareJune 20, 2022 19:10
@elvin-n

Copy link
Copy Markdown
ContributorAuthor

I am closing this PR because I split it to several ones:

  • PR11874 for handling of properly lowering the relay graph having memory scopes
  • PR11875 for changes in JSON file and support of new entites in graph executor
  • PR11876 for changes in static memory planner
  • PR11878 for Adreno specific markup pass annotating relay expr for memory scope to be handled in above PR

@elvin-n

Copy link
Copy Markdown
ContributorAuthor

@mbs-octoml - several more answers on initial comment

New annotator pass, which perhaps should also be made target-specific since the rules for deriving scope from shape seem pretty target specific.

The annotation pass consist of two parts - generic one and target specific. The generic one goes by graph and detects the targets and then construct name of the function including all found targets. It is not well robust for several targets, but works quite deterministic for one any target. We are introducing Adreno specific transformation, but other can be easily added. If we need to move adreno specific part into target specific directory, I will appreciate if you can suggest the proper place for target specific relay transformations.

I'm not sure how to reconcile the fairly generic multi-dimensional buffer support we now have with the hard 1d vs 2d distinction you're adding in graph executor. Is there a discussion about that someplace I've missed?

There is no fairly generic multi-dimensional buffer in the memory planner - memory manager operates for now by continuous 1d memory blocks or other words flatten 1d buffers. Later on in tir this flatten memory can be managed as multidimensional array using tvm arithmetic. But on the memory management stage it is represented always as 1d memory. Until this PR.

I can't quite tell if your StorageInfo pass supports tuples & tuple projection. That whole pass needs commenting. But in any case worth mentioning you've eschewed the collect-and-solve constraint approach used by plan devices in favor of an eager back propagation of scopes from argument to constants. Fair enough, but please document the subset of relay this supports.

It is still open question how to deal with tuple, I need some examples of ops producing tuples and not be merged into prim function. Could you please share which ops generate tuple of tensors as its output?

@elvin-nelvin-n closed this Jun 24, 2022
junrushao pushed a commit that referenced this pull request Jul 8, 2022
This PR is a split part of origin PR #11357
Co-authored-by: Chris Sullivan <csullivan@octoml.ai>
masahi pushed a commit to masahi/tvm that referenced this pull request Jul 15, 2022
This PR is a split part of origin PR apache#11357
Co-authored-by: Chris Sullivan <csullivan@octoml.ai>
junrushao pushed a commit to yelite/tvm that referenced this pull request Jul 27, 2022
This PR is a split part of origin PR apache#11357
Co-authored-by: Chris Sullivan <csullivan@octoml.ai>
xinetzone pushed a commit to daobook/tvm that referenced this pull request Nov 25, 2022
This PR is a split part of origin PR apache#11357
Co-authored-by: Chris Sullivan <csullivan@octoml.ai>
mikeseven pushed a commit to mikeseven/tvm that referenced this pull request Sep 27, 2023
This PR is a split part of origin PR apache#11357
Co-authored-by: Chris Sullivan <csullivan@octoml.ai>
Sign up for freeto 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.

3 participants

@elvin-n@csullivan@mbs-octoml
, 'i'); if (__m === '*' || __re.test(location.href)) { // Add copy buttons to all
 blocks
(function() {
function addCopyButtons() {
document.querySelectorAll('pre code').forEach(function(codeBlock) {
if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;
codeBlock.parentElement.setAttribute('data-copy-added', 'true');
var btn = document.createElement('button');
btn.textContent = 'Copy';
btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';
btn.onmouseover = function() { this.style.opacity = '1'; };
btn.onmouseout = function() { this.style.opacity = '0.7'; };
btn.onclick = function() {
navigator.clipboard.writeText(codeBlock.textContent).then(function() {
btn.textContent = 'Copied!';
setTimeout(function() { btn.textContent = 'Copy'; }, 1500);
});
};
codeBlock.parentElement.style.position = 'relative';
codeBlock.parentElement.appendChild(btn);
});
}
addCopyButtons();
// Re-run on dynamic content
var observer = new MutationObserver(addCopyButtons);
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
[Adreno] Enable static texture planning for constant data by elvin-n · Pull Request #11357 · apache/tvm · GitHub
Skip to content

[Adreno] Enable static texture planning for constant data - #11357

Closed
elvin-n wants to merge 4 commits into
apache:mainfrom
Deelvin:scout/adreno_static_textures
Closed

[Adreno] Enable static texture planning for constant data#11357
elvin-n wants to merge 4 commits into
apache:mainfrom
Deelvin:scout/adreno_static_textures

Conversation

@elvin-n

Copy link
Copy Markdown
Contributor

Previous PR11161 added topi compute and schedules which manages textures in dynamic mode through explicit call of cache_read in adreno primitive schedules. This PR adds top-down approach of memory annotation plus adding support in dependent parts.

  1. Passes of analyzing memory scope for expressions in the relay graph and modifies relay graph by adding/modifying VirutalDevice with required memory scope
  2. Add support of memory scope into graph memory planner
  3. Support of static planned textures in json and opencl runtime
  4. Modification of Relay->TIR settling taking into account relay expr virtual devices for variables

@elvin-n
elvin-nforce-pushed the scout/adreno_static_textures branch 3 times, most recently from 35aec2e to 467d321CompareMay 24, 2022 12:42
@elvin-n
elvin-n marked this pull request as ready for review May 24, 2022 12:58
@elvin-n

Copy link
Copy Markdown
ContributorAuthor

@csullivan@mbs-octoml PR is ready for review

@mbs-octomlmbs-octoml left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks, this is quite the heroic piece of work.

Can I suggest splitting this into smaller chunks?

  • Plumbing mem scope through TE (and perhaps handle the mem-scope-per-output issue)
  • Mem scope in graph executor mem planner, codegen & runtime.
  • 1D & 2D mem pools in graph executor runtime
  • Schedules
  • New annotator pass, which perhaps should also be made target-specific since the rules for deriving scope from shape seem pretty target specific. Ie it should be moved to a target specific dir, and the pass can be the identity if the current target does not match. Unfortunately there is no generic mechanism for registering a target-specific pass that will work for this.

I'm not sure how to reconcile the fairly generic multi-dimensional buffer support we now have with the hard 1d vs 2d distinction you're adding in graph executor. Is there a discussion about that someplace I've missed?

I can't quite tell if your StorageInfo pass supports tuples & tuple projection. That whole pass needs commenting. But in any case worth mentioning you've eschewed the collect-and-solve constraint approach used by plan devices in favor of an eager back propagation of scopes from argument to constants. Fair enough, but please document the subset of relay this supports.

Comment threadsrc/driver/driver_api.cc Outdated
primitive_supports_texture_ = false;
Visit(call->op);
if (primitive_supports_texture_) {
if (call->checked_type().as<TensorTypeNode>()) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

See tvm::relay::FlattenTupleType

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

this is done by intend so far - if we use FlattenTupleType, we would analyze each tensor individually that is wrong for this case. Currently I have an impression that if we have tuple been planned in static memory planner, the memory scope should be unique for all tensor contained in tuple

Comment threadsrc/relay/transforms/annotate_texture_storage.cc
Comment threadinclude/tvm/te/operation.h Outdated
@elvin-n
elvin-nforce-pushed the scout/adreno_static_textures branch from f13a402 to 23e5d7fCompareJune 20, 2022 19:04
Co-authored-by: Chris Sullivan <csullivan@octoml.ai>
@elvin-n
elvin-nforce-pushed the scout/adreno_static_textures branch from 23e5d7f to b73066bCompareJune 20, 2022 19:08
@elvin-n
elvin-nforce-pushed the scout/adreno_static_textures branch from b73066b to 02bc11cCompareJune 20, 2022 19:10
@elvin-n

Copy link
Copy Markdown
ContributorAuthor

I am closing this PR because I split it to several ones:

  • PR11874 for handling of properly lowering the relay graph having memory scopes
  • PR11875 for changes in JSON file and support of new entites in graph executor
  • PR11876 for changes in static memory planner
  • PR11878 for Adreno specific markup pass annotating relay expr for memory scope to be handled in above PR

@elvin-n

Copy link
Copy Markdown
ContributorAuthor

@mbs-octoml - several more answers on initial comment

New annotator pass, which perhaps should also be made target-specific since the rules for deriving scope from shape seem pretty target specific.

The annotation pass consist of two parts - generic one and target specific. The generic one goes by graph and detects the targets and then construct name of the function including all found targets. It is not well robust for several targets, but works quite deterministic for one any target. We are introducing Adreno specific transformation, but other can be easily added. If we need to move adreno specific part into target specific directory, I will appreciate if you can suggest the proper place for target specific relay transformations.

I'm not sure how to reconcile the fairly generic multi-dimensional buffer support we now have with the hard 1d vs 2d distinction you're adding in graph executor. Is there a discussion about that someplace I've missed?

There is no fairly generic multi-dimensional buffer in the memory planner - memory manager operates for now by continuous 1d memory blocks or other words flatten 1d buffers. Later on in tir this flatten memory can be managed as multidimensional array using tvm arithmetic. But on the memory management stage it is represented always as 1d memory. Until this PR.

I can't quite tell if your StorageInfo pass supports tuples & tuple projection. That whole pass needs commenting. But in any case worth mentioning you've eschewed the collect-and-solve constraint approach used by plan devices in favor of an eager back propagation of scopes from argument to constants. Fair enough, but please document the subset of relay this supports.

It is still open question how to deal with tuple, I need some examples of ops producing tuples and not be merged into prim function. Could you please share which ops generate tuple of tensors as its output?

@elvin-nelvin-n closed this Jun 24, 2022
junrushao pushed a commit that referenced this pull request Jul 8, 2022
This PR is a split part of origin PR #11357
Co-authored-by: Chris Sullivan <csullivan@octoml.ai>
masahi pushed a commit to masahi/tvm that referenced this pull request Jul 15, 2022
This PR is a split part of origin PR apache#11357
Co-authored-by: Chris Sullivan <csullivan@octoml.ai>
junrushao pushed a commit to yelite/tvm that referenced this pull request Jul 27, 2022
This PR is a split part of origin PR apache#11357
Co-authored-by: Chris Sullivan <csullivan@octoml.ai>
xinetzone pushed a commit to daobook/tvm that referenced this pull request Nov 25, 2022
This PR is a split part of origin PR apache#11357
Co-authored-by: Chris Sullivan <csullivan@octoml.ai>
mikeseven pushed a commit to mikeseven/tvm that referenced this pull request Sep 27, 2023
This PR is a split part of origin PR apache#11357
Co-authored-by: Chris Sullivan <csullivan@octoml.ai>
Sign up for freeto 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.

3 participants

@elvin-n@csullivan@mbs-octoml
, 'i'); if (__m === '*' || __re.test(location.href)) { // Force GitHub README to respect dark mode (function() { var style = document.createElement('style'); style.textContent = ' .markdown-body { color-scheme: dark light; } .markdown-body pre { background: #161b22 !important; } .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; } .markdown-body table th, .markdown-body table td { border-color: #30363d !important; } .markdown-body img { background: #0d1117; } .markdown-body blockquote { border-left-color: #8b949e; } .markdown-body hr { border-color: #30363d; } '; document.head.appendChild(style); })(); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' [Adreno] Enable static texture planning for constant data by elvin-n · Pull Request #11357 · apache/tvm · GitHub
Skip to content

[Adreno] Enable static texture planning for constant data - #11357

Closed
elvin-n wants to merge 4 commits into
apache:mainfrom
Deelvin:scout/adreno_static_textures
Closed

[Adreno] Enable static texture planning for constant data#11357
elvin-n wants to merge 4 commits into
apache:mainfrom
Deelvin:scout/adreno_static_textures

Conversation

@elvin-n

Copy link
Copy Markdown
Contributor

Previous PR11161 added topi compute and schedules which manages textures in dynamic mode through explicit call of cache_read in adreno primitive schedules. This PR adds top-down approach of memory annotation plus adding support in dependent parts.

  1. Passes of analyzing memory scope for expressions in the relay graph and modifies relay graph by adding/modifying VirutalDevice with required memory scope
  2. Add support of memory scope into graph memory planner
  3. Support of static planned textures in json and opencl runtime
  4. Modification of Relay->TIR settling taking into account relay expr virtual devices for variables

@elvin-n
elvin-nforce-pushed the scout/adreno_static_textures branch 3 times, most recently from 35aec2e to 467d321CompareMay 24, 2022 12:42
@elvin-n
elvin-n marked this pull request as ready for review May 24, 2022 12:58
@elvin-n

Copy link
Copy Markdown
ContributorAuthor

@csullivan@mbs-octoml PR is ready for review

@mbs-octomlmbs-octoml left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks, this is quite the heroic piece of work.

Can I suggest splitting this into smaller chunks?

  • Plumbing mem scope through TE (and perhaps handle the mem-scope-per-output issue)
  • Mem scope in graph executor mem planner, codegen & runtime.
  • 1D & 2D mem pools in graph executor runtime
  • Schedules
  • New annotator pass, which perhaps should also be made target-specific since the rules for deriving scope from shape seem pretty target specific. Ie it should be moved to a target specific dir, and the pass can be the identity if the current target does not match. Unfortunately there is no generic mechanism for registering a target-specific pass that will work for this.

I'm not sure how to reconcile the fairly generic multi-dimensional buffer support we now have with the hard 1d vs 2d distinction you're adding in graph executor. Is there a discussion about that someplace I've missed?

I can't quite tell if your StorageInfo pass supports tuples & tuple projection. That whole pass needs commenting. But in any case worth mentioning you've eschewed the collect-and-solve constraint approach used by plan devices in favor of an eager back propagation of scopes from argument to constants. Fair enough, but please document the subset of relay this supports.

Comment threadsrc/driver/driver_api.cc Outdated
primitive_supports_texture_ = false;
Visit(call->op);
if (primitive_supports_texture_) {
if (call->checked_type().as<TensorTypeNode>()) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

See tvm::relay::FlattenTupleType

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

this is done by intend so far - if we use FlattenTupleType, we would analyze each tensor individually that is wrong for this case. Currently I have an impression that if we have tuple been planned in static memory planner, the memory scope should be unique for all tensor contained in tuple

Comment threadsrc/relay/transforms/annotate_texture_storage.cc
Comment threadinclude/tvm/te/operation.h Outdated
@elvin-n
elvin-nforce-pushed the scout/adreno_static_textures branch from f13a402 to 23e5d7fCompareJune 20, 2022 19:04
Co-authored-by: Chris Sullivan <csullivan@octoml.ai>
@elvin-n
elvin-nforce-pushed the scout/adreno_static_textures branch from 23e5d7f to b73066bCompareJune 20, 2022 19:08
@elvin-n
elvin-nforce-pushed the scout/adreno_static_textures branch from b73066b to 02bc11cCompareJune 20, 2022 19:10
@elvin-n

Copy link
Copy Markdown
ContributorAuthor

I am closing this PR because I split it to several ones:

  • PR11874 for handling of properly lowering the relay graph having memory scopes
  • PR11875 for changes in JSON file and support of new entites in graph executor
  • PR11876 for changes in static memory planner
  • PR11878 for Adreno specific markup pass annotating relay expr for memory scope to be handled in above PR

@elvin-n

Copy link
Copy Markdown
ContributorAuthor

@mbs-octoml - several more answers on initial comment

New annotator pass, which perhaps should also be made target-specific since the rules for deriving scope from shape seem pretty target specific.

The annotation pass consist of two parts - generic one and target specific. The generic one goes by graph and detects the targets and then construct name of the function including all found targets. It is not well robust for several targets, but works quite deterministic for one any target. We are introducing Adreno specific transformation, but other can be easily added. If we need to move adreno specific part into target specific directory, I will appreciate if you can suggest the proper place for target specific relay transformations.

I'm not sure how to reconcile the fairly generic multi-dimensional buffer support we now have with the hard 1d vs 2d distinction you're adding in graph executor. Is there a discussion about that someplace I've missed?

There is no fairly generic multi-dimensional buffer in the memory planner - memory manager operates for now by continuous 1d memory blocks or other words flatten 1d buffers. Later on in tir this flatten memory can be managed as multidimensional array using tvm arithmetic. But on the memory management stage it is represented always as 1d memory. Until this PR.

I can't quite tell if your StorageInfo pass supports tuples & tuple projection. That whole pass needs commenting. But in any case worth mentioning you've eschewed the collect-and-solve constraint approach used by plan devices in favor of an eager back propagation of scopes from argument to constants. Fair enough, but please document the subset of relay this supports.

It is still open question how to deal with tuple, I need some examples of ops producing tuples and not be merged into prim function. Could you please share which ops generate tuple of tensors as its output?

@elvin-nelvin-n closed this Jun 24, 2022
junrushao pushed a commit that referenced this pull request Jul 8, 2022
This PR is a split part of origin PR #11357
Co-authored-by: Chris Sullivan <csullivan@octoml.ai>
masahi pushed a commit to masahi/tvm that referenced this pull request Jul 15, 2022
This PR is a split part of origin PR apache#11357
Co-authored-by: Chris Sullivan <csullivan@octoml.ai>
junrushao pushed a commit to yelite/tvm that referenced this pull request Jul 27, 2022
This PR is a split part of origin PR apache#11357
Co-authored-by: Chris Sullivan <csullivan@octoml.ai>
xinetzone pushed a commit to daobook/tvm that referenced this pull request Nov 25, 2022
This PR is a split part of origin PR apache#11357
Co-authored-by: Chris Sullivan <csullivan@octoml.ai>
mikeseven pushed a commit to mikeseven/tvm that referenced this pull request Sep 27, 2023
This PR is a split part of origin PR apache#11357
Co-authored-by: Chris Sullivan <csullivan@octoml.ai>
Sign up for freeto 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.

3 participants

@elvin-n@csullivan@mbs-octoml
, 'i'); if (__m === '*' || __re.test(location.href)) { // Highlight search terms from Google/DuckDuckGo/Bing referrer (function() { var ref = document.referrer; var terms = []; if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) { var url = new URL(ref); var q = url.searchParams.get('q') || url.searchParams.get('p'); if (q) { terms = q.split(/\s+/).filter(function(t) { return t.length > 2; }); } } if (terms.length === 0) return; var style = document.createElement('style'); style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }'; document.head.appendChild(style); function highlight(node) { if (node.nodeType === 3) { // text node var text = node.textContent; var found = false; terms.forEach(function(term) { var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\]\\]/g, '\\') + ')', 'gi'); if (regex.test(text)) { found = true; var frag = document.createDocumentFragment(); var parts = text.split(regex); parts.forEach(function(part, i) { if (i % 2 === 0) { frag.appendChild(document.createTextNode(part)); } else { var span = document.createElement('span'); span.className = 'userscript-highlight'; span.textContent = part; frag.appendChild(span); } }); node.parentNode.replaceChild(frag, node); } }); } else if (node.nodeType === 1 && node.childNodes) { // element var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT']; if (!skipTags.includes(node.tagName)) { Array.from(node.childNodes).forEach(highlight); } } } highlight(document.body); // Re-highlight on dynamic content var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1 || node.nodeType === 3) highlight(node); }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' [Adreno] Enable static texture planning for constant data by elvin-n · Pull Request #11357 · apache/tvm · GitHub
Skip to content

[Adreno] Enable static texture planning for constant data - #11357

Closed
elvin-n wants to merge 4 commits into
apache:mainfrom
Deelvin:scout/adreno_static_textures
Closed

[Adreno] Enable static texture planning for constant data#11357
elvin-n wants to merge 4 commits into
apache:mainfrom
Deelvin:scout/adreno_static_textures

Conversation

@elvin-n

Copy link
Copy Markdown
Contributor

Previous PR11161 added topi compute and schedules which manages textures in dynamic mode through explicit call of cache_read in adreno primitive schedules. This PR adds top-down approach of memory annotation plus adding support in dependent parts.

  1. Passes of analyzing memory scope for expressions in the relay graph and modifies relay graph by adding/modifying VirutalDevice with required memory scope
  2. Add support of memory scope into graph memory planner
  3. Support of static planned textures in json and opencl runtime
  4. Modification of Relay->TIR settling taking into account relay expr virtual devices for variables

@elvin-n
elvin-nforce-pushed the scout/adreno_static_textures branch 3 times, most recently from 35aec2e to 467d321CompareMay 24, 2022 12:42
@elvin-n
elvin-n marked this pull request as ready for review May 24, 2022 12:58
@elvin-n

Copy link
Copy Markdown
ContributorAuthor

@csullivan@mbs-octoml PR is ready for review

@mbs-octomlmbs-octoml left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks, this is quite the heroic piece of work.

Can I suggest splitting this into smaller chunks?

  • Plumbing mem scope through TE (and perhaps handle the mem-scope-per-output issue)
  • Mem scope in graph executor mem planner, codegen & runtime.
  • 1D & 2D mem pools in graph executor runtime
  • Schedules
  • New annotator pass, which perhaps should also be made target-specific since the rules for deriving scope from shape seem pretty target specific. Ie it should be moved to a target specific dir, and the pass can be the identity if the current target does not match. Unfortunately there is no generic mechanism for registering a target-specific pass that will work for this.

I'm not sure how to reconcile the fairly generic multi-dimensional buffer support we now have with the hard 1d vs 2d distinction you're adding in graph executor. Is there a discussion about that someplace I've missed?

I can't quite tell if your StorageInfo pass supports tuples & tuple projection. That whole pass needs commenting. But in any case worth mentioning you've eschewed the collect-and-solve constraint approach used by plan devices in favor of an eager back propagation of scopes from argument to constants. Fair enough, but please document the subset of relay this supports.

Comment threadsrc/driver/driver_api.cc Outdated
primitive_supports_texture_ = false;
Visit(call->op);
if (primitive_supports_texture_) {
if (call->checked_type().as<TensorTypeNode>()) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

See tvm::relay::FlattenTupleType

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

this is done by intend so far - if we use FlattenTupleType, we would analyze each tensor individually that is wrong for this case. Currently I have an impression that if we have tuple been planned in static memory planner, the memory scope should be unique for all tensor contained in tuple

Comment threadsrc/relay/transforms/annotate_texture_storage.cc
Comment threadinclude/tvm/te/operation.h Outdated
@elvin-n
elvin-nforce-pushed the scout/adreno_static_textures branch from f13a402 to 23e5d7fCompareJune 20, 2022 19:04
Co-authored-by: Chris Sullivan <csullivan@octoml.ai>
@elvin-n
elvin-nforce-pushed the scout/adreno_static_textures branch from 23e5d7f to b73066bCompareJune 20, 2022 19:08
@elvin-n
elvin-nforce-pushed the scout/adreno_static_textures branch from b73066b to 02bc11cCompareJune 20, 2022 19:10
@elvin-n

Copy link
Copy Markdown
ContributorAuthor

I am closing this PR because I split it to several ones:

  • PR11874 for handling of properly lowering the relay graph having memory scopes
  • PR11875 for changes in JSON file and support of new entites in graph executor
  • PR11876 for changes in static memory planner
  • PR11878 for Adreno specific markup pass annotating relay expr for memory scope to be handled in above PR

@elvin-n

Copy link
Copy Markdown
ContributorAuthor

@mbs-octoml - several more answers on initial comment

New annotator pass, which perhaps should also be made target-specific since the rules for deriving scope from shape seem pretty target specific.

The annotation pass consist of two parts - generic one and target specific. The generic one goes by graph and detects the targets and then construct name of the function including all found targets. It is not well robust for several targets, but works quite deterministic for one any target. We are introducing Adreno specific transformation, but other can be easily added. If we need to move adreno specific part into target specific directory, I will appreciate if you can suggest the proper place for target specific relay transformations.

I'm not sure how to reconcile the fairly generic multi-dimensional buffer support we now have with the hard 1d vs 2d distinction you're adding in graph executor. Is there a discussion about that someplace I've missed?

There is no fairly generic multi-dimensional buffer in the memory planner - memory manager operates for now by continuous 1d memory blocks or other words flatten 1d buffers. Later on in tir this flatten memory can be managed as multidimensional array using tvm arithmetic. But on the memory management stage it is represented always as 1d memory. Until this PR.

I can't quite tell if your StorageInfo pass supports tuples & tuple projection. That whole pass needs commenting. But in any case worth mentioning you've eschewed the collect-and-solve constraint approach used by plan devices in favor of an eager back propagation of scopes from argument to constants. Fair enough, but please document the subset of relay this supports.

It is still open question how to deal with tuple, I need some examples of ops producing tuples and not be merged into prim function. Could you please share which ops generate tuple of tensors as its output?

@elvin-nelvin-n closed this Jun 24, 2022
junrushao pushed a commit that referenced this pull request Jul 8, 2022
This PR is a split part of origin PR #11357
Co-authored-by: Chris Sullivan <csullivan@octoml.ai>
masahi pushed a commit to masahi/tvm that referenced this pull request Jul 15, 2022
This PR is a split part of origin PR apache#11357
Co-authored-by: Chris Sullivan <csullivan@octoml.ai>
junrushao pushed a commit to yelite/tvm that referenced this pull request Jul 27, 2022
This PR is a split part of origin PR apache#11357
Co-authored-by: Chris Sullivan <csullivan@octoml.ai>
xinetzone pushed a commit to daobook/tvm that referenced this pull request Nov 25, 2022
This PR is a split part of origin PR apache#11357
Co-authored-by: Chris Sullivan <csullivan@octoml.ai>
mikeseven pushed a commit to mikeseven/tvm that referenced this pull request Sep 27, 2023
This PR is a split part of origin PR apache#11357
Co-authored-by: Chris Sullivan <csullivan@octoml.ai>
Sign up for freeto 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.

3 participants

@elvin-n@csullivan@mbs-octoml
, 'i'); if (__m === '*' || __re.test(location.href)) { // Strip utm_, fbclid, gclid, etc. from all links on page (function() { var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content', 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid', 'ref', 'ref_src', 'source', 'medium', 'campaign']; function cleanUrl(url) { try { var u = new URL(url, window.location.origin); var changed = false; trackingParams.forEach(function(p) { if (u.searchParams.has(p)) { u.searchParams.delete(p); changed = true; } }); return changed ? u.toString() : url; } catch (e) { return url; } } function cleanLinks() { document.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } cleanLinks(); var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1) { if (node.tagName === 'A') cleanLinks(); node.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + ' [Adreno] Enable static texture planning for constant data by elvin-n · Pull Request #11357 · apache/tvm · GitHub
Skip to content

[Adreno] Enable static texture planning for constant data - #11357

Closed
elvin-n wants to merge 4 commits into
apache:mainfrom
Deelvin:scout/adreno_static_textures
Closed

[Adreno] Enable static texture planning for constant data#11357
elvin-n wants to merge 4 commits into
apache:mainfrom
Deelvin:scout/adreno_static_textures

Conversation

@elvin-n

Copy link
Copy Markdown
Contributor

Previous PR11161 added topi compute and schedules which manages textures in dynamic mode through explicit call of cache_read in adreno primitive schedules. This PR adds top-down approach of memory annotation plus adding support in dependent parts.

  1. Passes of analyzing memory scope for expressions in the relay graph and modifies relay graph by adding/modifying VirutalDevice with required memory scope
  2. Add support of memory scope into graph memory planner
  3. Support of static planned textures in json and opencl runtime
  4. Modification of Relay->TIR settling taking into account relay expr virtual devices for variables

@elvin-n
elvin-nforce-pushed the scout/adreno_static_textures branch 3 times, most recently from 35aec2e to 467d321CompareMay 24, 2022 12:42
@elvin-n
elvin-n marked this pull request as ready for review May 24, 2022 12:58
@elvin-n

Copy link
Copy Markdown
ContributorAuthor

@csullivan@mbs-octoml PR is ready for review

@mbs-octomlmbs-octoml left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks, this is quite the heroic piece of work.

Can I suggest splitting this into smaller chunks?

  • Plumbing mem scope through TE (and perhaps handle the mem-scope-per-output issue)
  • Mem scope in graph executor mem planner, codegen & runtime.
  • 1D & 2D mem pools in graph executor runtime
  • Schedules
  • New annotator pass, which perhaps should also be made target-specific since the rules for deriving scope from shape seem pretty target specific. Ie it should be moved to a target specific dir, and the pass can be the identity if the current target does not match. Unfortunately there is no generic mechanism for registering a target-specific pass that will work for this.

I'm not sure how to reconcile the fairly generic multi-dimensional buffer support we now have with the hard 1d vs 2d distinction you're adding in graph executor. Is there a discussion about that someplace I've missed?

I can't quite tell if your StorageInfo pass supports tuples & tuple projection. That whole pass needs commenting. But in any case worth mentioning you've eschewed the collect-and-solve constraint approach used by plan devices in favor of an eager back propagation of scopes from argument to constants. Fair enough, but please document the subset of relay this supports.

Comment threadsrc/driver/driver_api.cc Outdated
primitive_supports_texture_ = false;
Visit(call->op);
if (primitive_supports_texture_) {
if (call->checked_type().as<TensorTypeNode>()) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

See tvm::relay::FlattenTupleType

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

this is done by intend so far - if we use FlattenTupleType, we would analyze each tensor individually that is wrong for this case. Currently I have an impression that if we have tuple been planned in static memory planner, the memory scope should be unique for all tensor contained in tuple

Comment threadsrc/relay/transforms/annotate_texture_storage.cc
Comment threadinclude/tvm/te/operation.h Outdated
@elvin-n
elvin-nforce-pushed the scout/adreno_static_textures branch from f13a402 to 23e5d7fCompareJune 20, 2022 19:04
Co-authored-by: Chris Sullivan <csullivan@octoml.ai>
@elvin-n
elvin-nforce-pushed the scout/adreno_static_textures branch from 23e5d7f to b73066bCompareJune 20, 2022 19:08
@elvin-n
elvin-nforce-pushed the scout/adreno_static_textures branch from b73066b to 02bc11cCompareJune 20, 2022 19:10
@elvin-n

Copy link
Copy Markdown
ContributorAuthor

I am closing this PR because I split it to several ones:

  • PR11874 for handling of properly lowering the relay graph having memory scopes
  • PR11875 for changes in JSON file and support of new entites in graph executor
  • PR11876 for changes in static memory planner
  • PR11878 for Adreno specific markup pass annotating relay expr for memory scope to be handled in above PR

@elvin-n

Copy link
Copy Markdown
ContributorAuthor

@mbs-octoml - several more answers on initial comment

New annotator pass, which perhaps should also be made target-specific since the rules for deriving scope from shape seem pretty target specific.

The annotation pass consist of two parts - generic one and target specific. The generic one goes by graph and detects the targets and then construct name of the function including all found targets. It is not well robust for several targets, but works quite deterministic for one any target. We are introducing Adreno specific transformation, but other can be easily added. If we need to move adreno specific part into target specific directory, I will appreciate if you can suggest the proper place for target specific relay transformations.

I'm not sure how to reconcile the fairly generic multi-dimensional buffer support we now have with the hard 1d vs 2d distinction you're adding in graph executor. Is there a discussion about that someplace I've missed?

There is no fairly generic multi-dimensional buffer in the memory planner - memory manager operates for now by continuous 1d memory blocks or other words flatten 1d buffers. Later on in tir this flatten memory can be managed as multidimensional array using tvm arithmetic. But on the memory management stage it is represented always as 1d memory. Until this PR.

I can't quite tell if your StorageInfo pass supports tuples & tuple projection. That whole pass needs commenting. But in any case worth mentioning you've eschewed the collect-and-solve constraint approach used by plan devices in favor of an eager back propagation of scopes from argument to constants. Fair enough, but please document the subset of relay this supports.

It is still open question how to deal with tuple, I need some examples of ops producing tuples and not be merged into prim function. Could you please share which ops generate tuple of tensors as its output?

@elvin-nelvin-n closed this Jun 24, 2022
junrushao pushed a commit that referenced this pull request Jul 8, 2022
This PR is a split part of origin PR #11357
Co-authored-by: Chris Sullivan <csullivan@octoml.ai>
masahi pushed a commit to masahi/tvm that referenced this pull request Jul 15, 2022
This PR is a split part of origin PR apache#11357
Co-authored-by: Chris Sullivan <csullivan@octoml.ai>
junrushao pushed a commit to yelite/tvm that referenced this pull request Jul 27, 2022
This PR is a split part of origin PR apache#11357
Co-authored-by: Chris Sullivan <csullivan@octoml.ai>
xinetzone pushed a commit to daobook/tvm that referenced this pull request Nov 25, 2022
This PR is a split part of origin PR apache#11357
Co-authored-by: Chris Sullivan <csullivan@octoml.ai>
mikeseven pushed a commit to mikeseven/tvm that referenced this pull request Sep 27, 2023
This PR is a split part of origin PR apache#11357
Co-authored-by: Chris Sullivan <csullivan@octoml.ai>
Sign up for freeto 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.

3 participants

@elvin-n@csullivan@mbs-octoml
, 'i'); if (__m === '*' || __re.test(location.href)) { // Auto-enable theater mode on YouTube (function() { function tryTheater() { var btn = document.querySelector('button[aria-label="Theater mode"], ytd-player #player button[title="Theater mode"]'); if (btn && !btn.classList.contains('activated')) { btn.click(); } } // Try immediately tryTheater(); // Try after navigation (SPA) var lastUrl = location.href; setInterval(function() { if (location.href !== lastUrl) { lastUrl = location.href; setTimeout(tryTheater, 500); } }, 1000); // Also try on player load var observer = new MutationObserver(tryTheater); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' [Adreno] Enable static texture planning for constant data by elvin-n · Pull Request #11357 · apache/tvm · GitHub
Skip to content

[Adreno] Enable static texture planning for constant data - #11357

Closed
elvin-n wants to merge 4 commits into
apache:mainfrom
Deelvin:scout/adreno_static_textures
Closed

[Adreno] Enable static texture planning for constant data#11357
elvin-n wants to merge 4 commits into
apache:mainfrom
Deelvin:scout/adreno_static_textures

Conversation

@elvin-n

Copy link
Copy Markdown
Contributor

Previous PR11161 added topi compute and schedules which manages textures in dynamic mode through explicit call of cache_read in adreno primitive schedules. This PR adds top-down approach of memory annotation plus adding support in dependent parts.

  1. Passes of analyzing memory scope for expressions in the relay graph and modifies relay graph by adding/modifying VirutalDevice with required memory scope
  2. Add support of memory scope into graph memory planner
  3. Support of static planned textures in json and opencl runtime
  4. Modification of Relay->TIR settling taking into account relay expr virtual devices for variables

@elvin-n
elvin-nforce-pushed the scout/adreno_static_textures branch 3 times, most recently from 35aec2e to 467d321CompareMay 24, 2022 12:42
@elvin-n
elvin-n marked this pull request as ready for review May 24, 2022 12:58
@elvin-n

Copy link
Copy Markdown
ContributorAuthor

@csullivan@mbs-octoml PR is ready for review

@mbs-octomlmbs-octoml left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks, this is quite the heroic piece of work.

Can I suggest splitting this into smaller chunks?

  • Plumbing mem scope through TE (and perhaps handle the mem-scope-per-output issue)
  • Mem scope in graph executor mem planner, codegen & runtime.
  • 1D & 2D mem pools in graph executor runtime
  • Schedules
  • New annotator pass, which perhaps should also be made target-specific since the rules for deriving scope from shape seem pretty target specific. Ie it should be moved to a target specific dir, and the pass can be the identity if the current target does not match. Unfortunately there is no generic mechanism for registering a target-specific pass that will work for this.

I'm not sure how to reconcile the fairly generic multi-dimensional buffer support we now have with the hard 1d vs 2d distinction you're adding in graph executor. Is there a discussion about that someplace I've missed?

I can't quite tell if your StorageInfo pass supports tuples & tuple projection. That whole pass needs commenting. But in any case worth mentioning you've eschewed the collect-and-solve constraint approach used by plan devices in favor of an eager back propagation of scopes from argument to constants. Fair enough, but please document the subset of relay this supports.

Comment threadsrc/driver/driver_api.cc Outdated
primitive_supports_texture_ = false;
Visit(call->op);
if (primitive_supports_texture_) {
if (call->checked_type().as<TensorTypeNode>()) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

See tvm::relay::FlattenTupleType

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

this is done by intend so far - if we use FlattenTupleType, we would analyze each tensor individually that is wrong for this case. Currently I have an impression that if we have tuple been planned in static memory planner, the memory scope should be unique for all tensor contained in tuple

Comment threadsrc/relay/transforms/annotate_texture_storage.cc
Comment threadinclude/tvm/te/operation.h Outdated
@elvin-n
elvin-nforce-pushed the scout/adreno_static_textures branch from f13a402 to 23e5d7fCompareJune 20, 2022 19:04
Co-authored-by: Chris Sullivan <csullivan@octoml.ai>
@elvin-n
elvin-nforce-pushed the scout/adreno_static_textures branch from 23e5d7f to b73066bCompareJune 20, 2022 19:08
@elvin-n
elvin-nforce-pushed the scout/adreno_static_textures branch from b73066b to 02bc11cCompareJune 20, 2022 19:10
@elvin-n

Copy link
Copy Markdown
ContributorAuthor

I am closing this PR because I split it to several ones:

  • PR11874 for handling of properly lowering the relay graph having memory scopes
  • PR11875 for changes in JSON file and support of new entites in graph executor
  • PR11876 for changes in static memory planner
  • PR11878 for Adreno specific markup pass annotating relay expr for memory scope to be handled in above PR

@elvin-n

Copy link
Copy Markdown
ContributorAuthor

@mbs-octoml - several more answers on initial comment

New annotator pass, which perhaps should also be made target-specific since the rules for deriving scope from shape seem pretty target specific.

The annotation pass consist of two parts - generic one and target specific. The generic one goes by graph and detects the targets and then construct name of the function including all found targets. It is not well robust for several targets, but works quite deterministic for one any target. We are introducing Adreno specific transformation, but other can be easily added. If we need to move adreno specific part into target specific directory, I will appreciate if you can suggest the proper place for target specific relay transformations.

I'm not sure how to reconcile the fairly generic multi-dimensional buffer support we now have with the hard 1d vs 2d distinction you're adding in graph executor. Is there a discussion about that someplace I've missed?

There is no fairly generic multi-dimensional buffer in the memory planner - memory manager operates for now by continuous 1d memory blocks or other words flatten 1d buffers. Later on in tir this flatten memory can be managed as multidimensional array using tvm arithmetic. But on the memory management stage it is represented always as 1d memory. Until this PR.

I can't quite tell if your StorageInfo pass supports tuples & tuple projection. That whole pass needs commenting. But in any case worth mentioning you've eschewed the collect-and-solve constraint approach used by plan devices in favor of an eager back propagation of scopes from argument to constants. Fair enough, but please document the subset of relay this supports.

It is still open question how to deal with tuple, I need some examples of ops producing tuples and not be merged into prim function. Could you please share which ops generate tuple of tensors as its output?

@elvin-nelvin-n closed this Jun 24, 2022
junrushao pushed a commit that referenced this pull request Jul 8, 2022
This PR is a split part of origin PR #11357
Co-authored-by: Chris Sullivan <csullivan@octoml.ai>
masahi pushed a commit to masahi/tvm that referenced this pull request Jul 15, 2022
This PR is a split part of origin PR apache#11357
Co-authored-by: Chris Sullivan <csullivan@octoml.ai>
junrushao pushed a commit to yelite/tvm that referenced this pull request Jul 27, 2022
This PR is a split part of origin PR apache#11357
Co-authored-by: Chris Sullivan <csullivan@octoml.ai>
xinetzone pushed a commit to daobook/tvm that referenced this pull request Nov 25, 2022
This PR is a split part of origin PR apache#11357
Co-authored-by: Chris Sullivan <csullivan@octoml.ai>
mikeseven pushed a commit to mikeseven/tvm that referenced this pull request Sep 27, 2023
This PR is a split part of origin PR apache#11357
Co-authored-by: Chris Sullivan <csullivan@octoml.ai>
Sign up for freeto 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.

3 participants

@elvin-n@csullivan@mbs-octoml
, 'i'); if (__m === '*' || __re.test(location.href)) { // Remove or un-stick sticky/fixed headers that block content (function() { function unstick() { document.querySelectorAll('header, nav, [role="banner"], .header, .navbar, .sticky, .fixed-top, [style*="position: fixed"], [style*="position:sticky"]').forEach(function(el) { if (el.style.position === 'fixed' || el.style.position === 'sticky' || getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') { el.style.position = 'static'; el.style.top = 'auto'; el.style.zIndex = 'auto'; } }); } unstick(); var observer = new MutationObserver(unstick); observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] }); })(); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); })(); [Adreno] Enable static texture planning for constant data by elvin-n · Pull Request #11357 · apache/tvm · GitHub
Skip to content

[Adreno] Enable static texture planning for constant data - #11357

Closed
elvin-n wants to merge 4 commits into
apache:mainfrom
Deelvin:scout/adreno_static_textures
Closed

[Adreno] Enable static texture planning for constant data#11357
elvin-n wants to merge 4 commits into
apache:mainfrom
Deelvin:scout/adreno_static_textures

Conversation

@elvin-n

Copy link
Copy Markdown
Contributor

Previous PR11161 added topi compute and schedules which manages textures in dynamic mode through explicit call of cache_read in adreno primitive schedules. This PR adds top-down approach of memory annotation plus adding support in dependent parts.

  1. Passes of analyzing memory scope for expressions in the relay graph and modifies relay graph by adding/modifying VirutalDevice with required memory scope
  2. Add support of memory scope into graph memory planner
  3. Support of static planned textures in json and opencl runtime
  4. Modification of Relay->TIR settling taking into account relay expr virtual devices for variables

@elvin-n
elvin-nforce-pushed the scout/adreno_static_textures branch 3 times, most recently from 35aec2e to 467d321CompareMay 24, 2022 12:42
@elvin-n
elvin-n marked this pull request as ready for review May 24, 2022 12:58
@elvin-n

Copy link
Copy Markdown
ContributorAuthor

@csullivan@mbs-octoml PR is ready for review

@mbs-octomlmbs-octoml left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks, this is quite the heroic piece of work.

Can I suggest splitting this into smaller chunks?

  • Plumbing mem scope through TE (and perhaps handle the mem-scope-per-output issue)
  • Mem scope in graph executor mem planner, codegen & runtime.
  • 1D & 2D mem pools in graph executor runtime
  • Schedules
  • New annotator pass, which perhaps should also be made target-specific since the rules for deriving scope from shape seem pretty target specific. Ie it should be moved to a target specific dir, and the pass can be the identity if the current target does not match. Unfortunately there is no generic mechanism for registering a target-specific pass that will work for this.

I'm not sure how to reconcile the fairly generic multi-dimensional buffer support we now have with the hard 1d vs 2d distinction you're adding in graph executor. Is there a discussion about that someplace I've missed?

I can't quite tell if your StorageInfo pass supports tuples & tuple projection. That whole pass needs commenting. But in any case worth mentioning you've eschewed the collect-and-solve constraint approach used by plan devices in favor of an eager back propagation of scopes from argument to constants. Fair enough, but please document the subset of relay this supports.

Comment threadsrc/driver/driver_api.cc Outdated
primitive_supports_texture_ = false;
Visit(call->op);
if (primitive_supports_texture_) {
if (call->checked_type().as<TensorTypeNode>()) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

See tvm::relay::FlattenTupleType

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

this is done by intend so far - if we use FlattenTupleType, we would analyze each tensor individually that is wrong for this case. Currently I have an impression that if we have tuple been planned in static memory planner, the memory scope should be unique for all tensor contained in tuple

Comment threadsrc/relay/transforms/annotate_texture_storage.cc
Comment threadinclude/tvm/te/operation.h Outdated
@elvin-n
elvin-nforce-pushed the scout/adreno_static_textures branch from f13a402 to 23e5d7fCompareJune 20, 2022 19:04
Co-authored-by: Chris Sullivan <csullivan@octoml.ai>
@elvin-n
elvin-nforce-pushed the scout/adreno_static_textures branch from 23e5d7f to b73066bCompareJune 20, 2022 19:08
@elvin-n
elvin-nforce-pushed the scout/adreno_static_textures branch from b73066b to 02bc11cCompareJune 20, 2022 19:10
@elvin-n

Copy link
Copy Markdown
ContributorAuthor

I am closing this PR because I split it to several ones:

  • PR11874 for handling of properly lowering the relay graph having memory scopes
  • PR11875 for changes in JSON file and support of new entites in graph executor
  • PR11876 for changes in static memory planner
  • PR11878 for Adreno specific markup pass annotating relay expr for memory scope to be handled in above PR

@elvin-n

Copy link
Copy Markdown
ContributorAuthor

@mbs-octoml - several more answers on initial comment

New annotator pass, which perhaps should also be made target-specific since the rules for deriving scope from shape seem pretty target specific.

The annotation pass consist of two parts - generic one and target specific. The generic one goes by graph and detects the targets and then construct name of the function including all found targets. It is not well robust for several targets, but works quite deterministic for one any target. We are introducing Adreno specific transformation, but other can be easily added. If we need to move adreno specific part into target specific directory, I will appreciate if you can suggest the proper place for target specific relay transformations.

I'm not sure how to reconcile the fairly generic multi-dimensional buffer support we now have with the hard 1d vs 2d distinction you're adding in graph executor. Is there a discussion about that someplace I've missed?

There is no fairly generic multi-dimensional buffer in the memory planner - memory manager operates for now by continuous 1d memory blocks or other words flatten 1d buffers. Later on in tir this flatten memory can be managed as multidimensional array using tvm arithmetic. But on the memory management stage it is represented always as 1d memory. Until this PR.

I can't quite tell if your StorageInfo pass supports tuples & tuple projection. That whole pass needs commenting. But in any case worth mentioning you've eschewed the collect-and-solve constraint approach used by plan devices in favor of an eager back propagation of scopes from argument to constants. Fair enough, but please document the subset of relay this supports.

It is still open question how to deal with tuple, I need some examples of ops producing tuples and not be merged into prim function. Could you please share which ops generate tuple of tensors as its output?

@elvin-nelvin-n closed this Jun 24, 2022
junrushao pushed a commit that referenced this pull request Jul 8, 2022
This PR is a split part of origin PR #11357
Co-authored-by: Chris Sullivan <csullivan@octoml.ai>
masahi pushed a commit to masahi/tvm that referenced this pull request Jul 15, 2022
This PR is a split part of origin PR apache#11357
Co-authored-by: Chris Sullivan <csullivan@octoml.ai>
junrushao pushed a commit to yelite/tvm that referenced this pull request Jul 27, 2022
This PR is a split part of origin PR apache#11357
Co-authored-by: Chris Sullivan <csullivan@octoml.ai>
xinetzone pushed a commit to daobook/tvm that referenced this pull request Nov 25, 2022
This PR is a split part of origin PR apache#11357
Co-authored-by: Chris Sullivan <csullivan@octoml.ai>
mikeseven pushed a commit to mikeseven/tvm that referenced this pull request Sep 27, 2023
This PR is a split part of origin PR apache#11357
Co-authored-by: Chris Sullivan <csullivan@octoml.ai>
Sign up for freeto 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.

3 participants

@elvin-n@csullivan@mbs-octoml