This is an archive of the discontinued LLVM Phabricator instance.

[PowerPC] Fix use of FPSCR builtins in smmintrin.h
AbandonedPublic

Authored by qiucf on Aug 16 2023, 3:08 AM.

Details

Reviewers
nemanjai
shchenz
stefanp
Group Reviewers
Restricted Project
Summary

smmintrin.h uses __builtin_mffs, __builtin_mffsl, __builtin_mtfsf and __builtin_set_fpscr_rn. This patch replaces the uses with ppc prefix and implement the missing ones.

This fixes issue 64664.

Diff Detail

Event Timeline

qiucf created this revision.Aug 16 2023, 3:08 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 16 2023, 3:08 AM
Herald added a subscriber: kbarton. · View Herald Transcript
qiucf requested review of this revision.Aug 16 2023, 3:08 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 16 2023, 3:08 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript
tuliom added a subscriber: tuliom.Aug 16 2023, 12:12 PM

Are there any plans to align the names of these built-ins with GCC? Their built-ins do not have the _ppc part.

CC @amyk @quinnp Any comments about the naming?

I see some __builtin_ppc_xxx are aliased into __builtin_xxx by defineXLCompatMacros. But these are not XL-compatible builtins, and the macros do not always work.

CC @amyk @quinnp Any comments about the naming?

I see some __builtin_ppc_xxx are aliased into __builtin_xxx by defineXLCompatMacros. But these are not XL-compatible builtins, and the macros do not always work.

It should be perfectly fine to provide pre-defined macros for these to match GCC on PowerPC. The reason we went with the macro solution is to avoid polluting the builtins namespace for other targets.

Also, please add some C++ tests for these PPC wrappers so that we aren't surprised again when someone tries to use these in their C++ code.

It should be perfectly fine to provide pre-defined macros for these to match GCC on PowerPC. The reason we went with the macro solution is to avoid polluting the builtins namespace for other targets.

Also, please add some C++ tests for these PPC wrappers so that we aren't surprised again when someone tries to use these in their C++ code.

It seems these macros do not always work for all PowerPC targets:

// -target=ppc64le -mcpu=power9 do not work
// -target=ppc64le-unknown-linux-gnu -mcpu=power9 work
long calldarn(void) { return __darn(); }
qiucf updated this revision to Diff 555805.Sep 4 2023, 10:40 PM
  • Add macro alias to mffs mffsl mtfsf and set_fpscr_rn, although they don't work in freestanding mode
  • Add C++ run lines to intrinsics tests. To avoid further messing codegen checks, make them run under -fsyntax-only.