Skip to content

[RISCV] Better run-time detection of RVV vector instruction support#1770

Merged
Dead2 merged 1 commit intozlib-ng:developfrom
mtl1979:riscv-detect
Sep 12, 2024
Merged

[RISCV] Better run-time detection of RVV vector instruction support#1770
Dead2 merged 1 commit intozlib-ng:developfrom
mtl1979:riscv-detect

Conversation

@mtl1979
Copy link
Copy Markdown
Collaborator

@mtl1979 mtl1979 commented Aug 26, 2024

Original version posted by @ncopa in #1705.

@mtl1979
Copy link
Copy Markdown
Collaborator Author

mtl1979 commented Aug 26, 2024

@alexsifivetw Can you tag some RISC-V users who could test or review the PR...

@codecov
Copy link
Copy Markdown

codecov bot commented Aug 26, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 83.02%. Comparing base (5b04d9c) to head (6d966d4).
Report is 5 commits behind head on develop.

Additional details and impacted files
@@           Coverage Diff            @@
##           develop    #1770   +/-   ##
========================================
  Coverage    83.02%   83.02%           
========================================
  Files          135      135           
  Lines        10310    10310           
  Branches      2785     2785           
========================================
  Hits          8560     8560           
+ Misses        1057     1056    -1     
- Partials       693      694    +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Copy Markdown

@alexsifivetw alexsifivetw left a comment

Choose a reason for hiding this comment

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

LGTM.

@nmoinvaz nmoinvaz added the Architecture Architecture specific label Aug 29, 2024
if (uname(&buffer) == -1) {
// uname failed
return 0;
}
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.

Only way this can fail is if the host name is too long, usually longer than 64 characters reserved for each of the fields in the struct. Even then some versions of uname function will just truncate the host name and leave NULL-termination out. However if -1 is returned, the other fields are guaranteed to be uninitialised.

Not ignoring the return value makes it clear that the call to uname and buffer in stack can't be optimised out, which would cause the contents of buffer later below to be uninitialised. Some compilers will issue warning if the contents of struct member can be uninitialised (not guaranteed to be zero-filled), but that might require enabling "higher" warning level or static analysis.

@Dead2
Copy link
Copy Markdown
Member

Dead2 commented Aug 31, 2024

@mtl1979 Remember to take this out of Draft status when you feel it is ready.

@mtl1979
Copy link
Copy Markdown
Collaborator Author

mtl1979 commented Aug 31, 2024

@mtl1979 Remember to take this out of Draft status when you feel it is ready.

I still have one pending issue where the fix likely overlaps the same area of code... I rather combine everything to single PR so I don't have to revise this one right after...

@mtl1979
Copy link
Copy Markdown
Collaborator Author

mtl1979 commented Sep 11, 2024

I haven't seen any activity on #1769 for two weeks now, so I'm not delaying this PR for longer... I'm not completely sure if it even is issue in zlib-ng or the way the binary itself, or C library and kernel is compiled.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Architecture Architecture specific

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants