Skip to content

[SM6.10] LinAlg Validation: Validate Params and K Dim#8588

Open
V-FEXrt wants to merge 6 commits into
microsoft:mainfrom
V-FEXrt:linalg-validation-no-undef
Open

[SM6.10] LinAlg Validation: Validate Params and K Dim#8588
V-FEXrt wants to merge 6 commits into
microsoft:mainfrom
V-FEXrt:linalg-validation-no-undef

Conversation

@V-FEXrt

@V-FEXrt V-FEXrt commented Jun 23, 2026

Copy link
Copy Markdown
Collaborator

Add the following validation rules:

  • undef is disallowed as a LinAlg builtin parameter
  • LinAlg types without metadata are disallowed as LinAlg builtin parameters and return types
  • LinAlg type returned from a LinAlg builtin must have a valid K dimension

The vast majority of the change is updating tests that were previously out of spec as well as adding new tests that intentionally raise the new validation failures.

Some of the tests are fixed by using the fillmatrix builtin to have an actual matrix SSA value. This will be invalid in some cases but those invalid uses will be much easier to fix as they are found/disallowed by future validation PRs

@bob80905 bob80905 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think we should add a test that checks for the new diagnostic.

@github-actions

github-actions Bot commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

✅ With the latest revision this PR passed the C/C++ code formatter.

@V-FEXrt V-FEXrt changed the title [SM6.10] LinAlg Validation: Disallow undef as builtin params [SM6.10] LinAlg Validation: Validate Params and K dim Jun 29, 2026
@V-FEXrt V-FEXrt changed the title [SM6.10] LinAlg Validation: Validate Params and K dim [SM6.10] LinAlg Validation: Validate Params and K Dim Jun 29, 2026
Comment thread lib/DxilValidation/DxilValidationUtils.cpp Outdated

@bob80905 bob80905 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

My other comment is a nit, it can be taken or left. Otherwise, LGTM

@damyanp damyanp linked an issue Jul 1, 2026 that may be closed by this pull request
Comment thread lib/DxilValidation/DxilValidation.cpp Outdated
Comment thread lib/DxilValidation/DxilValidation.cpp Outdated
Comment thread lib/DxilValidation/DxilValidation.cpp Outdated
Comment thread lib/DxilValidation/DxilValidationUtils.h Outdated
Comment thread lib/DxilValidation/DxilValidationUtils.h Outdated
Comment thread lib/DxilValidation/DxilValidationUtils.cpp Outdated
Comment on lines +55 to +75
ConstantInt *Ints[5];

for (size_t I = 0; I < 5; ++I) {
ConstantAsMetadata *ConstMDI =
dyn_cast<ConstantAsMetadata>(MDT->getOperand(I + 1).get());
if (!ConstMDI)
return std::nullopt;
ConstantInt *CI = dyn_cast<ConstantInt>(ConstMDI->getValue());
if (!CI)
return std::nullopt;
Ints[I] = CI;
}

LinAlgTargetType LATT;
LATT.Type = static_cast<DXIL::ComponentType>(Ints[0]->getLimitedValue());
LATT.M = Ints[1]->getLimitedValue();
LATT.N = Ints[2]->getLimitedValue();
LATT.Use = static_cast<DXIL::MatrixUse>(Ints[3]->getLimitedValue());
LATT.Scope = static_cast<DXIL::MatrixScope>(Ints[4]->getLimitedValue());

return {{Ty, LATT}};

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

(read the next comment first please ;))

This is want I have in mind:

Suggested change
ConstantInt *Ints[5];
for (size_t I = 0; I < 5; ++I) {
ConstantAsMetadata *ConstMDI =
dyn_cast<ConstantAsMetadata>(MDT->getOperand(I + 1).get());
if (!ConstMDI)
return std::nullopt;
ConstantInt *CI = dyn_cast<ConstantInt>(ConstMDI->getValue());
if (!CI)
return std::nullopt;
Ints[I] = CI;
}
LinAlgTargetType LATT;
LATT.Type = static_cast<DXIL::ComponentType>(Ints[0]->getLimitedValue());
LATT.M = Ints[1]->getLimitedValue();
LATT.N = Ints[2]->getLimitedValue();
LATT.Use = static_cast<DXIL::MatrixUse>(Ints[3]->getLimitedValue());
LATT.Scope = static_cast<DXIL::MatrixScope>(Ints[4]->getLimitedValue());
return {{Ty, LATT}};
uint64_t Ints[5];
for (size_t I = 0; I < 5; ++I) {
ConstantAsMetadata *ConstMDI =
dyn_cast<ConstantAsMetadata>(MDT->getOperand(I + 1).get());
if (!ConstMDI)
return;
ConstantInt *CI = dyn_cast<ConstantInt>(ConstMDI->getValue());
if (!CI)
return;
Ints[I] = CI->getLimitedValue();
}
auto Result = Map.try_emplace(Ty, static_cast<DXIL::ComponentType>(Ints[0]),
Ints[1], Ints[2], static_cast<DXIL::MatrixUse>(Ints[3]),
static_cast<DXIL::MatrixScope>(Ints[4]));
if (!Result.second)
;// TODO: validation error - metadata for this type already exists

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

how strongly do you want the validation error for type already exists?

The map is built up during initialization/before we have access to a ValidationContext (in fact this function call is in the ValContext ctor). If we want to raise an error there, we'll need to do some rearchitecting somewhere.

Comment thread lib/DxilValidation/DxilValidationUtils.cpp Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: New

Development

Successfully merging this pull request may close these issues.

LinAlg Validation: Validate Matrix K Dimension

4 participants