-
Notifications
You must be signed in to change notification settings - Fork 7
Add Windows pipeline & Sync with upstream code #5
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
Conversation
| PackedQuantBScale = (float*)((std::byte*)QuantBBlkSum + BlkSumSize); | ||
| QuantBBlkSum = (T*)(PackedQuantBData + PackedQuantBDataSize); | ||
| QuantBBlkSum = (T*)MlasAlignAddress(QuantBBlkSum, MlasQNBitQuantBBlkSumAlignment()); | ||
| PackedQuantBScale = (T*)((std::byte*)QuantBBlkSum + BlkSumSize); |
Check failure
Code scanning / CodeQL
Suspicious pointer scaling
| { | ||
|
|
||
| enum SQNBitGemmVariant { | ||
| enum QNBitGemmVariant { |
Check notice
Code scanning / CodeQL
Irregular enum initialization
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 2 months ago
To fix the irregular enum initialization, we should explicitly initialize all enum members in QNBitGemmVariant. Their integer values should be consecutive starting from 0 for the valid variants (unless stated otherwise), matching the explicit initializations already present and ensuring that SQNBitGemmVariantCount remains equal to the number of valid variants. Specifically, assign sequential values to each variant, such that the relationship between name and value is explicit. This change should only occur within the definition of the QNBitGemmVariant enum in file src/lib/qnbitgemm.cpp, lines 26–41.
No additional imports or new methods are required, as this is a purely syntactic correction inside a C++ enum.
-
Copy modified lines R32-R34 -
Copy modified line R40
| @@ -29,15 +29,15 @@ | ||
| // Valid variants | ||
|
|
||
| SQNBitGemmVariant_BitWidth4_CompFp32 = 0, | ||
| SQNBitGemmVariant_BitWidth4_CompInt8, | ||
| HQNBitGemmVariant_BitWidth4_CompFp16, | ||
| HQNBitGemmVariant_BitWidth4_CompInt8, | ||
| SQNBitGemmVariant_BitWidth4_CompInt8 = 1, | ||
| HQNBitGemmVariant_BitWidth4_CompFp16 = 2, | ||
| HQNBitGemmVariant_BitWidth4_CompInt8 = 3, | ||
|
|
||
| // End of valid variants | ||
|
|
||
| // Keep this element last and ensure that its value is the number of valid QNBitGemmVariant values. | ||
| // Its value is used as an array size. | ||
| SQNBitGemmVariantCount, | ||
| SQNBitGemmVariantCount = 4, | ||
| }; | ||
|
|
||
| QNBitGemmVariant |
| switch (variant) { | ||
| case SQNBitGemmVariant_BitWidth4_CompInt8: | ||
| return InitializeWorkspace_CompInt8<float>; | ||
| default: | ||
| return nullptr; | ||
| } |
Check notice
Code scanning / CodeQL
No trivial switch statements
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 2 months ago
To address the "No trivial switch statements" error at line 755, we should replace the switch with an if/else statement. This entails checking if (variant == SQNBitGemmVariant_BitWidth4_CompInt8) and returning the appropriate function pointer, otherwise returning the default (nullptr). Only the block in the GetInitializeWorkspace<float> specialization (lines 753–761) needs to be rewritten. No changes are needed to imports, method signatures, or other parts of the code outside these lines. Functionality remains identical, as only the control structure is changed.
-
Copy modified lines R755-R758
| @@ -752,11 +752,10 @@ | ||
| InitializeWorkspaceFn<float> | ||
| GetInitializeWorkspace(QNBitGemmVariant variant) | ||
| { | ||
| switch (variant) { | ||
| case SQNBitGemmVariant_BitWidth4_CompInt8: | ||
| return InitializeWorkspace_CompInt8<float>; | ||
| default: | ||
| return nullptr; | ||
| if (variant == SQNBitGemmVariant_BitWidth4_CompInt8) { | ||
| return InitializeWorkspace_CompInt8<float>; | ||
| } else { | ||
| return nullptr; | ||
| } | ||
| } | ||
|
|
| switch (variant) { | ||
| case HQNBitGemmVariant_BitWidth4_CompInt8: | ||
| return InitializeWorkspace_CompInt8<MLAS_FP16>; | ||
| default: | ||
| return nullptr; | ||
| } |
Check notice
Code scanning / CodeQL
No trivial switch statements
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 2 months ago
The best way to fix this is to replace the trivial switch statement in GetInitializeWorkspace<MLAS_FP16> (lines 767–772) with a simple if/else statement. Specifically, the code should use an if to check for equality with HQNBitGemmVariant_BitWidth4_CompInt8, returning the corresponding function pointer when true, and returning nullptr otherwise. No additional imports or new method/definition is needed; all functionality and logic stay identical, just reformatted for clarity and to follow static analysis guidelines.
Only the code within the function GetInitializeWorkspace<MLAS_FP16> (lines 767–772) should be changed.
-
Copy modified lines R767-R770
| @@ -764,11 +764,10 @@ | ||
| InitializeWorkspaceFn<MLAS_FP16> | ||
| GetInitializeWorkspace(QNBitGemmVariant variant) | ||
| { | ||
| switch (variant) { | ||
| case HQNBitGemmVariant_BitWidth4_CompInt8: | ||
| return InitializeWorkspace_CompInt8<MLAS_FP16>; | ||
| default: | ||
| return nullptr; | ||
| if (variant == HQNBitGemmVariant_BitWidth4_CompInt8) { | ||
| return InitializeWorkspace_CompInt8<MLAS_FP16>; | ||
| } else { | ||
| return nullptr; | ||
| } | ||
| } | ||
|
|
| switch (variant) { | ||
| case HQNBitGemmVariant_BitWidth4_CompFp16: | ||
| return HQ4BitGemm_CompFp16; | ||
| default: | ||
| return nullptr; | ||
| } |
Check notice
Code scanning / CodeQL
No trivial switch statements
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 2 months ago
To fix this true positive, the trivial switch statement in the function template specialization GetQNBitGemm<MLAS_FP16> should be replaced by an if/else statement. Specifically, the switch (variant) block on lines 809-814 should be transformed so that it checks if variant equals HQNBitGemmVariant_BitWidth4_CompFp16 and returns HQ4BitGemm_CompFp16 if so, otherwise returns nullptr. This can be done by replacing the switch block with a direct if/else, without altering any functionality. Only the lines from 809 (the switch line) to 814 (the closing brace of the function) should be modified.
No new imports or helper functions are needed for this change.
-
Copy modified lines R809-R812
| @@ -806,11 +806,10 @@ | ||
| QNBitGemmFn<MLAS_FP16> | ||
| GetQNBitGemm(QNBitGemmVariant variant) | ||
| { | ||
| switch (variant) { | ||
| case HQNBitGemmVariant_BitWidth4_CompFp16: | ||
| return HQ4BitGemm_CompFp16; | ||
| default: | ||
| return nullptr; | ||
| if (variant == HQNBitGemmVariant_BitWidth4_CompFp16) { | ||
| return HQ4BitGemm_CompFp16; | ||
| } else { | ||
| return nullptr; | ||
| } | ||
| } | ||
| } // namespace |
| uses: ./.github/workflows/reusable_windows_build.yml | ||
| with: | ||
| job-name: Win32_Debug_NoOrt_CodeQL | ||
| runner-os: windows-2022 | ||
| cmake-workflow-preset: windows_win32_debug_no_ort_workflow | ||
| enable-codeql: true | ||
|
|
||
| Win32_release_no_ort: |
Check warning
Code scanning / CodeQL
Workflow does not contain permissions Medium
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 2 months ago
To fix the problem, we should explicitly declare a permissions: block in the workflow file to restrict the permissions of the GITHUB_TOKEN. Since the jobs in this workflow appear to be running builds/tests and are not performing actions such as pushing to the repository or managing issues/pull requests, the minimum required privilege is likely contents: read. This permissions block should be added at the top level, just after the workflow name: and before any jobs, to apply to all jobs unless overridden. No changes to job definitions or behavior are required. Simply add the following block:
permissions:
contents: readNo imports or definitions are needed for this change.
-
Copy modified lines R2-R3
| @@ -1,4 +1,6 @@ | ||
| name: Windows_CI | ||
| permissions: | ||
| contents: read | ||
|
|
||
| on: | ||
| push: |
| uses: ./.github/workflows/reusable_windows_build.yml | ||
| with: | ||
| job-name: Win32_Release_NoOrt | ||
| runner-os: windows-2022 | ||
| cmake-workflow-preset: windows_win32_release_no_ort_workflow | ||
| enable-codeql: false | ||
|
|
||
| # WinX64 Jobs | ||
| WinX64_debug_no_ort: |
Check warning
Code scanning / CodeQL
Workflow does not contain permissions Medium
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 2 months ago
To fix the problem, add a permissions key at the workflow root to explicitly restrict the GITHUB_TOKEN permissions to the minimum required. As a starting point, permissions: {} (no permissions) can be used, which means no write access by default. If some jobs require read access to contents (which is needed for most workflows to check out code, etc.), use permissions: contents: read. Place this at the very top after the name: (line 2 or 3). If in the future more granular access is needed for specialized jobs (e.g., deploying, opening PRs), those job blocks can override the default with their own permissions as needed. In this file, simply insert the following near the top:
permissions: {}Or, as a safer default if you expect the jobs may need to checkout code (typical with GitHub Actions):
permissions:
contents: readThis should be immediately below the name: field and before the on: trigger.
-
Copy modified lines R2-R3
| @@ -1,4 +1,6 @@ | ||
| name: Windows_CI | ||
| permissions: | ||
| contents: read | ||
|
|
||
| on: | ||
| push: |
| uses: ./.github/workflows/reusable_windows_build.yml | ||
| with: | ||
| job-name: Winx64_Debug_NoOrt_CodeQL | ||
| runner-os: windows-2022 | ||
| cmake-workflow-preset: windows_x64_debug_no_ort_workflow | ||
| enable-codeql: true | ||
|
|
||
| WinX64_release_no_ort: |
Check warning
Code scanning / CodeQL
Workflow does not contain permissions Medium
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 2 months ago
To fix this problem, we need to explicitly set a permissions block in the workflow. This can be done either at the root of the YAML file to apply to all jobs, or per job for more granular control. If the jobs do not need to write to the repository and only require access for read operations (e.g., checking out code), the minimal permission set should be contents: read. Add the following block after the name: or before the on: trigger to restrict permissions for all jobs (as none have their own permissions blocks). This edit should be made to .github/workflows/win_ci.yml, immediately after the name: Windows_CI line. No imports or additional definitions are needed.
-
Copy modified lines R2-R3
| @@ -1,4 +1,6 @@ | ||
| name: Windows_CI | ||
| permissions: | ||
| contents: read | ||
|
|
||
| on: | ||
| push: |
| uses: ./.github/workflows/reusable_windows_build.yml | ||
| with: | ||
| job-name: Winx64_Release_NoOrt | ||
| runner-os: windows-2022 | ||
| cmake-workflow-preset: windows_x64_release_no_ort_workflow | ||
| enable-codeql: false | ||
|
|
||
| WinX64_release: |
Check warning
Code scanning / CodeQL
Workflow does not contain permissions Medium
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 2 months ago
General fix approach:
To resolve this, add a permissions block to the workflow at the top level, directly beneath the workflow name (or after on: if more conventional). This block should specify the least privilege needed by the workflow. If the jobs do not need to write to or interact with the repository via the GITHUB_TOKEN, the safest minimal setting is contents: read. If some jobs require further permissions, these can be overridden per-job as needed.
How to fix specifically:
- Edit
.github/workflows/win_ci.yml. - Insert the line
permissions:\n contents: readafter thename: Windows_CIline (line 1), so that it is applied workflow-wide for all jobs that do not override it. - No further import or method changes are necessary, as this is configuration only.
-
Copy modified lines R2-R3
| @@ -1,4 +1,6 @@ | ||
| name: Windows_CI | ||
| permissions: | ||
| contents: read | ||
|
|
||
| on: | ||
| push: |
| uses: ./.github/workflows/reusable_windows_build.yml | ||
| with: | ||
| job-name: Winx64_Release | ||
| runner-os: windows-2022 | ||
| cmake-workflow-preset: windows_x64_release_workflow | ||
| enable-codeql: false | ||
|
|
||
| # Windows ARM64 Jobs | ||
| WinARM64_debug_no_ort: |
Check warning
Code scanning / CodeQL
Workflow does not contain permissions Medium
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 2 months ago
The best way to fix this problem is to set an explicit permissions key with the minimal privileges required. If you're unsure about the required permissions, the most restrictive form is permissions: {} (which means no permissions), or, more commonly, permissions: read-all (contents: read). For most simple CI workflows that do not need to push code, create releases, or modify pull requests, contents: read is sufficient.
The fix should be applied at the root of the workflow (just below the name: and above or below on:) so that all jobs inherit it. In .github/workflows/win_ci.yml, add:
permissions:
contents: readimmediately after line 1 (the name). This is the recommended minimal starting point. If the workflow needs to interact with issues, PRs, etc., those can be incrementally added as necessary.
No additional imports, methods, or code changes are required for this fix.
-
Copy modified lines R2-R3
| @@ -1,4 +1,6 @@ | ||
| name: Windows_CI | ||
| permissions: | ||
| contents: read | ||
|
|
||
| on: | ||
| push: |
| uses: ./.github/workflows/reusable_windows_build.yml | ||
| with: | ||
| job-name: WinARM64_Debug_NoOrt | ||
| runner-os: windows-11-arm # Use ARM64 runner | ||
| cmake-workflow-preset: windows_arm64_debug_no_ort_workflow | ||
| enable-codeql: false | ||
|
|
||
| WinARM64_release: |
Check warning
Code scanning / CodeQL
Workflow does not contain permissions Medium
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 2 months ago
The best way to fix the problem is to set an explicit permissions block at the top/root of the workflow file (.github/workflows/win_ci.yml). This will ensure that the GITHUB_TOKEN given to all jobs in this workflow is limited to only the specified scopes. If jobs require very specific permissions (e.g., for uploading artifacts or managing PRs), those can be expanded, but the safe minimum starting point is contents: read (the default recommendation). Since the jobs in this workflow run builds/tests using a reusable workflow, but do not appear to modify contents, PRs, issues, or other repo resources, contents: read is the lowest sensible privilege. The change should be made by inserting:
permissions:
contents: readdirectly after the workflow name and before the on: trigger block (best practice), i.e., after line 1 or 2.
-
Copy modified lines R2-R3
| @@ -1,4 +1,6 @@ | ||
| name: Windows_CI | ||
| permissions: | ||
| contents: read | ||
|
|
||
| on: | ||
| push: |
| uses: ./.github/workflows/reusable_windows_build.yml | ||
| with: | ||
| job-name: WinARM64_Release | ||
| runner-os: windows-11-arm # Use ARM64 runner | ||
| cmake-workflow-preset: windows_arm64_release_workflow | ||
| enable-codeql: false No newline at end of file |
Check warning
Code scanning / CodeQL
Workflow does not contain permissions Medium
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 2 months ago
To fix the problem, explicitly set the permissions key at the top (root) level of the workflow file .github/workflows/win_ci.yml, which applies to all jobs unless individually overridden. The safest, least-privilege baseline is contents: read, as recommended by GitHub and various security best practices. This allows the workflow read-only access to the repository contents with the GITHUB_TOKEN, which is sufficient for most CI/test jobs that only need to clone and build the code but not push changes or interact with issues. No functional changes will occur to the build/test jobs. Insert the following block right after the name field and before on.
-
Copy modified lines R2-R3
| @@ -1,4 +1,6 @@ | ||
| name: Windows_CI | ||
| permissions: | ||
| contents: read | ||
|
|
||
| on: | ||
| push: |
Uh oh!
There was an error while loading. Please reload this page.