This is an archive of the discontinued LLVM Phabricator instance.

[PowerPC] Convert vec_splats functions to macros
Needs ReviewPublic

Authored by vddvss on May 28 2020, 7:15 AM.

Details

Reviewers
nemanjai
Group Reviewers
Restricted Project
Summary

This commit converts the vec_splats functions in altivec.h to macros, solving an issue where vec_splats calls could not assign to variables of static storage duration, such as:

static vector int x = vec_splats(1);

Since vec_splats was implemented as a function, code such as in this example would result in a compile-time error in clang. This differs from gcc, which allows this construct.

This updates tests accordingly, and fixes PR44276 and PR44455. Sorry for the delay in getting this to you.

Diff Detail

Event Timeline

vddvss created this revision.May 28 2020, 7:15 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 28 2020, 7:16 AM
steven.zhang added inline comments.
clang/lib/Headers/altivec.h
13670

I am not sure if this is by intention. It is not semantics the same with this change. Before the change, if VSX is off, and POWER8_VECTOR && powerpc64 is on, vector signed/unsigned long long, signed/unsigned __int128 is not a valid candidate of vec_splats. But with this patch, they are.

vddvss added inline comments.May 28 2020, 10:04 AM
clang/lib/Headers/altivec.h
13670

No intention to change semantics. But AFICT, we throw an error if POWER8_VECTOR is on and VSX is off: https://github.com/llvm/llvm-project/blob/master/clang/lib/Basic/Targets/PPC.cpp#L222

steven.zhang added inline comments.May 28 2020, 7:31 PM
clang/lib/Headers/altivec.h
13670

Hmm, we are making assumption that, POWER8_VECTOR enables the VSX, and it is true. Thank you for pointing out this.

clang/test/CodeGen/pr44276.c
4

The assembly output is not your test point. How about doing it as this:

// RUN: %clang_cc1 -S -emit-llvm -triple powerpc64-unknown-unknown -target-cpu pwr8 %s
// expected-no-diagnostics
vddvss updated this revision to Diff 267529.May 31 2020, 2:53 PM
vddvss marked an inline comment as done.

Updated revision to address @steven.zhang's good suggestion on the test case.

This also does clang-format on altivec.h to address the harbormaster failure, although I am inclined to do /* clang-format off */, since it is a lot less readable after running clang-format.

It LGTM now except one comment on the test. And it seems that, we still have many other builtins implementation that didn't use the _Generic.

clang/test/CodeGen/ppc-emmintrin.c
1

So, why this line of comments is removed ? It seems that, the old test was generated by the script while the new one isn't. I expect both should generate by script.