Skip to content

doc: LLVM mtune in 2025.10 Toolchain Only Supports RV32#60

Merged
fanghuaqi merged 2 commits intomasterfrom
nuclei/zfj_dev
Dec 23, 2025
Merged

doc: LLVM mtune in 2025.10 Toolchain Only Supports RV32#60
fanghuaqi merged 2 commits intomasterfrom
nuclei/zfj_dev

Conversation

@bigmagic123
Copy link
Contributor

@bigmagic123 bigmagic123 commented Nov 27, 2025

Important

Adds documentation for a known issue in LLVM 2025.10 regarding Nuclei -mtune support in intro.rst.

  • Documentation:
    • Adds a "Known Issues" section in intro.rst for LLVM 2025.10.
    • Describes a bug in Nuclei -mtune support where processor models use RISCVProcessorModel instead of RISCVTuneProcessorModel.
    • References GitHub issue for more details.

This description was created by Ellipsis for 756928e. You can customize this summary. It will automatically update as commits are pushed.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

Important

Looks good to me! 👍

Reviewed everything up to 7de1efd in 1 minute and 32 seconds. Click for details.
  • Reviewed 13 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 2 draft comments. View those below.
  • Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. source/toolchain/changelog.rst:53
  • Draft comment:
    Consider adding a space before the parenthesis for clarity (i.e., 'series (RV32 Only)').
  • Reason this comment was not posted:
    Confidence changes required: 33% <= threshold 50% None
2. source/toolchain/changelog.rst:53
  • Draft comment:
    Typographical suggestion: Consider adding a space before the opening parenthesis in "series(RV32 Only)" for better readability. For example, consider "series (RV32 Only)".
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 30% vs. threshold = 50% This is a very minor stylistic comment about spacing. While the suggestion might improve readability slightly, it's not a functional issue. The rules state "Do NOT make comments that are obvious or unimportant" and "Do NOT comment unless there is clearly a code change required." This is a documentation file (changelog.rst), and the spacing issue is extremely minor. The comment is technically correct - adding a space would follow common English typography conventions - but it's borderline trivial. However, for documentation, proper formatting can be important for professionalism. I need to weigh whether this rises to the level of "clearly a code change required" or if it's too minor. While proper spacing in documentation is good practice, this is an extremely minor stylistic issue that doesn't affect functionality or comprehension. The rules emphasize not making "obvious or unimportant" comments, and a single missing space in a changelog entry could be considered unimportant. However, this is a documentation file where readability and professional formatting matter. The comment is actionable with a clear suggestion, and proper spacing before parentheses is a standard typography convention. It's a quick fix that improves quality. This is a borderline case - it's a minor but valid stylistic improvement to documentation. Given the emphasis on not making unimportant comments and the trivial nature of a single space, I lean toward deleting this comment as it doesn't meet the threshold of "clearly a code change required."

Workflow ID: wflow_KhDEJn3eYY67bj06

You can customize Ellipsis by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.

@fanghuaqi
Copy link
Member

不是这么写,应该如实写清楚,这里的mtune实现有哪些问题。

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

Important

Looks good to me! 👍

Reviewed 756928e in 1 minute and 40 seconds. Click for details.
  • Reviewed 35 lines of code in 2 files
  • Skipped 0 files when reviewing.
  • Skipped posting 2 draft comments. View those below.
  • Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. source/toolchain/changelog.rst:53
  • Draft comment:
    Inconsistent architecture note: The '(RV32 Only)' tag was removed, but PR title indicates only RV32 is supported. Clarify if this change is correct.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50% This comment is explicitly asking to "Clarify if this change is correct" - this is a verification request to the PR author. According to the rules, I should NOT keep comments that ask the PR author to confirm their intention, explain, or double-check things. The comment doesn't point to a definite error; it's speculative based on information I cannot verify (the PR title). The author intentionally removed "(RV32 Only)", which suggests they meant to broaden the support or correct an inaccuracy. Without strong evidence that this removal is wrong, I should delete this comment. The comment might be catching a legitimate inconsistency if the PR title truly says "RV32 only" but the changelog now implies broader support. This could be a real documentation error that needs fixing. Even if there's a potential inconsistency with the PR title, the comment is asking for clarification rather than stating a definite problem. The rules explicitly say not to ask authors to verify or clarify. Additionally, I cannot see the PR title to verify the claim, and the rule states I should only keep comments with STRONG EVIDENCE of correctness. Without being able to verify the PR title claim, this is speculative. This comment should be deleted because it asks the PR author to clarify/verify the change rather than pointing to a definite error. It violates the rule against asking authors to confirm their intention or double-check things. Without access to the PR title to verify the claimed inconsistency, there's no strong evidence this comment is correct.
2. source/toolchain/llvm/intro.rst:159
  • Draft comment:
    The Known Issues section documents a bug with nuclei '-mtune' support. Ensure that details (e.g., impact on non-RV32 targets) are consistent with the changelog and PR title.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50% This comment is asking the PR author to ensure consistency between the Known Issues section, changelog, and PR title. It is not making a specific code suggestion or pointing out a specific issue in the code. It is more of a general request for consistency, which falls under the rule of not asking the author to ensure things are consistent or verified.

Workflow ID: wflow_w6KWNOBRWSWowh7p

You can customize Ellipsis by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.

@fanghuaqi fanghuaqi merged commit 8692063 into master Dec 23, 2025
1 check passed
@fanghuaqi fanghuaqi deleted the nuclei/zfj_dev branch December 23, 2025 03:00
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