This is an archive of the discontinued LLVM Phabricator instance.

[X86] Remove __builtin_ia32_pmax/min intrinsics and use generic __builtin_elementwise_max/min
ClosedPublic

Authored by RKSimon on Jan 20 2022, 9:08 AM.

Details

Summary

D111985 added the generic __builtin_elementwise_max and __builtin_elementwise_min intrinsics with the same integer behaviour as the SSE/AVX instructions

This patch removes the __builtin_ia32_pmax/min intrinsics and just uses __builtin_elementwise_max/min - the existing tests see no changes:

__m256i test_mm256_max_epu32(__m256i a, __m256i b) {
  // CHECK-LABEL: test_mm256_max_epu32
  // CHECK: call <8 x i32> @llvm.umax.v8i32(<8 x i32> %{{.*}}, <8 x i32> %{{.*}})
  return _mm256_max_epu32(a, b);
}

This requires us to add a __v64qs explicitly signed char vector type (we already have __v16qs and __v32qs).

Sibling patch to D117791

Diff Detail

Event Timeline

RKSimon requested review of this revision.Jan 20 2022, 9:08 AM
RKSimon created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptJan 20 2022, 9:08 AM
This revision is now accepted and ready to land.Jan 20 2022, 9:13 AM
pengfei accepted this revision.Jan 20 2022, 5:24 PM
pengfei added inline comments.
clang/lib/Headers/avx512bwintrin.h
894

Should we change the type here too? The same below.

Thanks!

clang/lib/Headers/avx512bwintrin.h
894

I don't think so - __builtin_ia32_selectb_512 uses __v64qi everywhere else, and ignores signedness. It only matters inside the min/max builtins

fhahn accepted this revision.Jan 21 2022, 2:50 AM

Thanks for the patch, it's great to see!

This revision was landed with ongoing or failed builds.Jan 21 2022, 4:25 AM
This revision was automatically updated to reflect the committed changes.