Page MenuHomePhabricator

[PPC] add float and double overloads for vec_orc and vec_nand in altivec.h
ClosedPublic

Authored by sfertile on Oct 25 2016, 10:53 AM.

Details

Summary

add missing overloads for vec_orc:
vector double vec_orc (vector bool long long, vector double);
vector double vec_orc (vector double, vector bool long long);
vector float vec_orc (vector bool int, vector float);
vector float vec_orc (vector float, vector bool int);

and vec_nand:
vector double vec_nand (vector double, vector double);
vector float vec_nand (vector float, vector float);

Diff Detail

Repository
rL LLVM

Event Timeline

sfertile updated this revision to Diff 75729.Oct 25 2016, 10:53 AM
sfertile retitled this revision from to [PPC] add float and double overloads for vec_orc and vec_nand in altivec.h.
sfertile updated this object.
sfertile set the repository for this revision to rL LLVM.
sfertile added subscribers: llvm-commits, echristo.
amehsan added inline comments.Oct 25 2016, 11:00 AM
lib/Headers/altivec.h
5293–5298

@kbarton Using C-style cast here, violates strict aliasing. Is that correct? Could it lead to functional problems if someone compiles their code WITHOUT -fno-strict-aliasing while using this built-ins? For example if this built-in is used in a context where a and b are deferenced pointers? Note that this will be inlined.

amehsan added inline comments.Oct 25 2016, 11:01 AM
lib/Headers/altivec.h
5293–5298

used in a context where

__a and __b

Sorry for bad format of the comment.
@nemanjai what do you think?

nemanjai added inline comments.Oct 25 2016, 1:47 PM
lib/Headers/altivec.h
5293–5298

I may be wrong, but I don't think it does still. I think the semantics of the cast are always equivalent to a static_cast rather than a reinterpret_cast.

echristo added inline comments.Oct 25 2016, 2:51 PM
lib/Headers/altivec.h
5293–5298

The semantics of the cast should be equivalent to static_cast and not reinterpret_cast. It's overall a weird set of fallback rules and there's really nothing to say what happens in the spec.

Also, FWIW, we do this all over the place in the x86 intrinsics.

amehsan added inline comments.Oct 25 2016, 5:38 PM
lib/Headers/altivec.h
5293–5298

Also, FWIW, we do this all over the place in the x86 intrinsics.

Good to know. Then we don't need to worry about this.

The semantics of the cast should be equivalent to static_cast and not reinterpret_cast.

I think what is meant here is that the cast here does not change the binary representation of the vector. Right? That seems to be the behavior both static_cast and reinterpret_cast for vectors:

amehsan@genoa:~/dev$ cat vcast.cpp
#include<altivec.h>
#include <iostream>
using namespace std;

int main() {

  vector float vf = {2.3f, 2.4f, 2.5f, 2.6f};
  cout << vf[1] << endl;
  vector unsigned vu = static_cast<vector unsigned>(vf);
  cout << vu[1] << endl;
  vector unsigned vu2 = reinterpret_cast<vector unsigned>(vf);
  cout << vu2[1] << endl;
  vector unsigned vu3 = (vector unsigned)vf;
  cout << vu3[1] << endl;

  float x = 3.14f;
  cout << x << endl;
  cout << static_cast<unsigned>(x) << endl;
  //cout << reinterpret_cast<unsigned>(x) << endl;//compile error
  cout << (unsigned) x << endl;

  return 0;

}
amehsan@genoa:~/dev$ clang++ vcast.cpp -o vcast  -maltivec
amehsan@genoa:~/dev$ ./vcast
2.4
1075419546
1075419546
1075419546
3.14
3
3
amehsan@genoa:~/dev$
nemanjai edited edge metadata.Oct 27 2016, 12:48 AM

This LGTM but I'll let @kbarton have a look as well.

kbarton accepted this revision.Oct 27 2016, 1:04 PM
kbarton edited edge metadata.

LGTM

This revision is now accepted and ready to land.Oct 27 2016, 1:04 PM
sfertile updated this revision to Diff 76230.Oct 28 2016, 12:07 PM
sfertile edited edge metadata.

rebased to top of master, picking up incoming altivec.h changes

Committed revision 285439.
Sean please close this review if there are no buildbot failures.

sfertile closed this revision.Oct 31 2016, 7:08 AM