Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

List of things to improve in the HLSL language #104

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

hekota
Copy link
Member

@hekota hekota commented Nov 27, 2024

This PR starts documenting a list of features that we might consider removing in a future version of the HLSL language. It includes things that:

  • have unusual syntax or odd behavior
  • are remnants of outdated features that are no longer used or relevant
  • are construct that are generally considered "not very nice" from a language point of view

This list may include potential solutions for each item. The purpose of this PR is solely to document the proposed options. The discussion on which solution is optimal can be postponed for future consideration.


### 1. Global variables outside of `cbuffer` context

All global variables declared outside of `cbuffer` context go implicitly into `$Globals` constant buffer. If the variable has an initializer it gets ignored.
Copy link
Collaborator

Choose a reason for hiding this comment

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

line wrapping


### 1. Global variables outside of `cbuffer` context

All global variables declared outside of `cbuffer` context go implicitly into `$Globals` constant buffer. If the variable has an initializer it gets ignored.
Copy link
Collaborator

Choose a reason for hiding this comment

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

If the variable has an initializer it gets ignored.

A little ambiguous whether "it" refers to the variable or the initializer. Maybe "Any initializers on global variables are ignored".

@@ -0,0 +1,69 @@
<!-- {% raw %} -->

# List of Things to Improve in the HLSL Language
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm wondering if a proposal document is the right way to capture this? Any reason not to use issues for each thing? Then a proposal document can be written with a concrete proposal on addressing the thing-that-needs-to-be-improved.

Copy link
Member Author

Choose a reason for hiding this comment

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

I've created this list based on a @llvm-beanz's suggestion that it would be helpful to have one place that lists all of the not-so-nice features and obscure constructs in the language. We can then refer to it when designing a new language version. Issues would probably too as long as they are labeled such that they're easy to find.


All global variables declared outside of `cbuffer` context go implicitly into `$Globals` constant buffer. If the variable has an initializer it gets ignored.

We should either require all global variables to be declared withing `cbuffer` context and stop supporting `$Globals`, or we should require any top-level non-static global variable to be declared with `const` and report error if it is initialized.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why are static global variables different here?

I'm wondering if we should avoid suggesting solutions in this document since debate over solutions might detract from the main purpose of it.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think it would be useful to list potential solutions without going to too much detail.

Copy link
Member Author

@hekota hekota Nov 28, 2024

Choose a reason for hiding this comment

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

I've removed the non-static. I've update it to list possible solution without going into too much of a detail and updated the PR description that this is not a place to discuss which solution is best.


All global variables declared outside of `cbuffer` context go implicitly into `$Globals` constant buffer. If the variable has an initializer it gets ignored.

We should either require all global variables to be declared withing `cbuffer` context and stop supporting `$Globals`, or we should require any top-level non-static global variable to be declared with `const` and report error if it is initialized.
Copy link
Collaborator

Choose a reason for hiding this comment

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

This suggested solution implies that the only issue with global variables outside of cbuffer is that initializers are ignored. Is that the primary thing we're trying to solve here, or are there more reasons that $Globals is undesireable?

Copy link
Member Author

Choose a reason for hiding this comment

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

@tex3d - would you have more to add here why we should get rid of $Globals constant buffer?


```
https://godbolt.org/z/PeW4E3aj3

Copy link
Collaborator

Choose a reason for hiding this comment

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

My 2c: we should get rid of cbuffer entirely and just use ConstantBuffer instead so we have more regular syntax for other types of buffers.

@llvm-beanz
Copy link
Collaborator

@hekota, thank you for starting to put this together!

Because this is language-related we should probably instead have this over in https://github.com/microsoft/hlsl-specs/.

I do wonder if this is something we should track as a proposal, or just as more general documentation though. Maybe we should start writing some docs around longer term language direction that we can collect in hlsl-specs?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

3 participants