Page MenuHomePhabricator

[X86] Update some av512 shift intrinsics to use "unsigned int" parameter instead of int to match Intel documentaiton
ClosedPublic

Authored by craig.topper on Tue, May 19, 2:51 PM.

Details

Summary

There are 65 that take a scalar shift amount. Intel documentation shows 60 of them taking unsigned int. There are 5 versions of srli_epi16 that use int, the 512-bit maskz and 128/256 mask/maskz.

Fixes PR45931

Diff Detail

Event Timeline

craig.topper created this revision.Tue, May 19, 2:51 PM

Is it possible to fix those other 5 in the Intel docs for consistency, or is there some functional reason that those are different?

Is it possible to fix those other 5 in the Intel docs for consistency, or is there some functional reason that those are different?

I think it was just a mistake. The docs are derived from icc's source code which also has them wrong. Note all of the SSE/AVX2 shift intrinsics use int not unsigned int.

Can we add -Wsign-conversion checks to the tests? That was mentioned on PR45931

Can we add -Wsign-conversion checks to the tests? That was mentioned on PR45931

I can. Will that do anything other than show that my test_* functions were updated to match the new type?

RKSimon accepted this revision.Fri, May 22, 8:31 AM

Can we add -Wsign-conversion checks to the tests? That was mentioned on PR45931

I can. Will that do anything other than show that my test_* functions were updated to match the new type?

not much other than check for regressions again tbh.

otherwise, lgtm

This revision is now accepted and ready to land.Fri, May 22, 8:31 AM

Add -Wsign-conversion. Fix two test issues that exposed.

This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptFri, May 22, 9:25 PM