Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

fix 128bits ctlz intrinsincs UB#635

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Open
dvermd wants to merge2 commits intorust-lang:master
base:master
Choose a base branch
Loading
fromdvermd:fix_128b_leading_zeros_ub

Conversation

dvermd
Copy link

Fixes#604

This is the GIMPLE code generated

      if (param0 == 0) goto then; else goto else;      then:      zeros = 128;      goto after;      else:      _2 = param0 >> 64;      _3 = (size_t) _2;      if (_3 != 0) goto ctlz_then; else goto ctlz_else;      after:      stack_var_1.15_4 = &stack_var_1;      MEM[(unsigned int *)stack_var_1.15_4] = zeros;      stack_var_1.16_5 = &stack_var_1;      loadedValue2 = MEM[(unsigned int *)stack_var_1.16_5];      D.4444 = loadedValue2;      return D.4444;      ctlz_then:      _6 = param0 >> 64;      _7 = (size_t) _6;      _8 = __builtin_clzll (_7);      zeros = (unsigned int) _8;      goto ctlz_after;      ctlz_else:      _9 = (size_t) param0;      _10 = __builtin_clzll (_9);      _11 = (unsigned int) _10;      zeros = _11 + 64;      goto ctlz_after;      ctlz_after:      zeros = zeros;      goto after;

@dvermd
Copy link
Author

It looks like the some CI jobs are failing on

thread 'rustc' panicked at src/int.rs:176:13:assertion failed: a_type.dyncast_array().is_some()

which is some code I didn't touch.

Can my changes have some side effects ?
I see that#631 also have these jobs failing. Perhaps it's not my changes which bring these failures.

Any hints ?

@antoyo
Copy link
Contributor

Not sure it's related. I opened a new dummyPR to check if the tests pass, though.

@antoyo
Copy link
Contributor

TheCI passed, so the problem is in here.

I suspect this is because you changed the return type: it used to return an array type and your change makes it return au32.

@dvermd
Copy link
Author

dvermd commentedFeb 23, 2025
edited
Loading

The return type of theif width == 128 was already au32 because the block ended with

                let res = self.context.new_array_access(self.location, result, index);                return self.gcc_int_cast(res.to_rvalue(), result_type);

which will get an element in the array and cast it as result_type which isu32.

Edit: formatting

@dvermd
Copy link
Author

Ok, finally found the line pointing in my changelet leading_zeroes = self.add(low_leading_zeroes, sixty_four); wherelow_leading_zeroesandsixty_four are of different types.

The problem I see is that theif branch should have been taken.

Copy link
Contributor

@antoyoantoyo left a comment

Choose a reason for hiding this comment

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

Sorry for the delay: I've been very busy.

let result = self.current_func()
.new_local(None, array_type, "count_loading_zeroes_results");

// Algorithm from: https://stackoverflow.com/a/28433850/389119 updated to check for high 64bits being 0
Copy link
Contributor

@antoyoantoyoMar 28, 2025
edited
Loading

Choose a reason for hiding this comment

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

It seems your new algorithm significantly diverges from the one linked here.
If so, could you please replace this link with another pointing to the algorithm you used? If you can't, could you please add comments below to make this easier to understand?

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

@antoyoantoyoantoyo left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

UB in the implementation of the 128 bit ctlz intrinsic
2 participants
@dvermd@antoyo

[8]ページ先頭

©2009-2025 Movatter.jp