Skip to content

[mlir][dxsa] Add root dxsa.module#155

Open
tagolog wants to merge 1 commit into
access-softek:dxsa-mlirfrom
tagolog:dxsa-mlir-program-header
Open

[mlir][dxsa] Add root dxsa.module#155
tagolog wants to merge 1 commit into
access-softek:dxsa-mlirfrom
tagolog:dxsa-mlir-program-header

Conversation

@tagolog
Copy link
Copy Markdown

@tagolog tagolog commented May 31, 2026

Wraps the program into a module with optional attributes program type and shader version. When the binary has no header both attributes are omitted.

Example:
dxsa.module pixel_shader 5 0 {
dxsa.dcl_global_flags
}

dxsa.module {
dxsa.dcl_global_flags
}

Wraps the program into a module with optional attributes program type and shader version.
When the binary has no header both attributes are omitted.

Example:
  dxsa.module pixel_shader 5 0 {
    dxsa.dcl_global_flags <refactoringAllowed>
  }

  dxsa.module {
    dxsa.dcl_global_flags <refactoringAllowed>
  }

Signed-off-by: Vladimir Shiryaev <tagolog@users.noreply.github.com>
@tagolog tagolog requested a review from asavonic May 31, 2026 08:02
Copy link
Copy Markdown
Contributor

@asavonic asavonic left a comment

Choose a reason for hiding this comment

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

LGTM overall.

parser.getContext(), *programType));

unsigned major = 0, minor = 0;
if (parser.parseInteger(major) || parser.parseInteger(minor))
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.

If we declare major and minor as uint8_t, then parseInteger is going to check that the value is in range.
We can also omit static_cast below.

}

ParseResult ModuleOp::parse(OpAsmParser &parser, OperationState &result) {
Region *body = result.addRegion();
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.

Let's move body initialization closer to its first use.


Module createModule(mlir::dxsa::ProgramTypeAttr programType,
mlir::dxsa::ShaderVersionAttr shaderVersion,
FileLineColLoc loc) {
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.

Should we use Location instead?

FileLineColLoc loc = getLocation(0);
std::vector<Instruction> instructions;
auto header = parseProgramHeader();
if (failed(header))
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.

FAILURE_IF_FAILED?

#include "llvm/Support/SourceMgr.h"

namespace mlir::dxsa {
/// Deserializes the given binary \p source and creates a MLIR ModuleOp in the given \p context.
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.

Clang-format?

Sign up for free to 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.

2 participants