Skip to content

Conversation

@zhaomaosu
Copy link
Contributor

@zhaomaosu zhaomaosu commented Nov 1, 2024

intel/llvm part: intel/llvm#15954

@zhaomaosu zhaomaosu requested a review from a team as a code owner November 1, 2024 03:02
@github-actions github-actions bot added loader Loader related feature/bug sanitizer Sanitizer layer issues/changes/specification labels Nov 1, 2024
@AllanZyne
Copy link
Contributor

AllanZyne commented Nov 4, 2024

Let's change the mapping logic of DG2 as well?

@zhaomaosu
Copy link
Contributor Author

Let's change the mapping logic of DG2 as well?

Done

// Set env AllocateHostAllocationsInHeapExtendedHost to 0, then host and
// shared usm will have same allocation range on GPU device. This will
// simplify shadow memory mapping logic on GPU device.
setenv("NEOReadDebugKeys", "1", 1);
Copy link
Contributor

Choose a reason for hiding this comment

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

@nrspruit is it a good idea to set these env variables in the layer? If this is really required, wouldn't it be a better idea to make this into an L0 API?
Using a debug key for functionality doesn't seem like a good idea.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't feel comfortable merging this patch. We need confirmation from the NEO folks that this makes sense.
@MichalMrozek can you help?

Copy link
Contributor

@MichalMrozek MichalMrozek Nov 26, 2024

Choose a reason for hiding this comment

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

No, this is not correct to set this.
Please do not merge it.
If you want to have specific release keys avaialable this needs discussion with driver team.

Copy link
Contributor

Choose a reason for hiding this comment

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

AllocateHostAllocationsInHeapExtendedHost is a debug key, debug keys cannot be used in production software. They are meant only for experiments. Driver team may remove or change this debug variable at any point of time without any notifications.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, I will send this request to driver team. Thanks.

@kbenzie kbenzie added v0.11.x Include in the v0.11.x release and removed v0.11.x Include in the v0.11.x release labels Nov 25, 2024
@zhaomaosu
Copy link
Contributor Author

I'll try to find other optimization opportunity. I'm closing this PR.

@zhaomaosu zhaomaosu closed this Dec 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

loader Loader related feature/bug sanitizer Sanitizer layer issues/changes/specification

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants