Promote bswap i16, i32, and i64 to be allowed llvm intrinsics.

Promote bswap i16, i32, and i64 to be allowed llvm intrinsics.

Disallow i1, i8, which don't make sense for "byte" swapping.
None of these generate outcalls to compiler_rt.

Test coverage:
* test/CodeGen/AArch64/dp1.ll (only testing i32, i64)
* test/CodeGen/ARM/rev.ll (only testing i16)
* test/CodeGen/Mips/bswap.ll (only testing i32, i64)
* test/CodeGen/X86/bswap.ll (i16, i32, i64, on i686)
* test/NaCl/{ARM,X86}/intrinsics-bitmanip.ll (i16, i32, i64)
(maybe the last set of tests could be merged into the
upstream tests)

For targets without native i16 bswap, the i16 could just be
a bswap on an i32 plus a shift right 16.

Other test coverage:
* gcc/testsuite/gcc.dg/builtin-bswap-[1,2,3,4,5].c

Misc: style cleanups, and add comments to the undocumented intrinsics in the dev list.

BUG=https://code.google.com/p/nativeclient/issues/detail?id=3378
R=dschuff@chromium.org

Committed: https://gerrit.chromium.org/gerrit/gitweb?p=native_client/pnacl-llvm.git;a=commit;h=5019000

[eliben, 2013-05-15 23:34:54]

https://codereview.chromium.org/14914011/diff/1/lib/Analysis/NaCl/PNaClABIVer...
File lib/Analysis/NaCl/PNaClABIVerifyModule.cpp (right):

https://codereview.chromium.org/14914011/diff/1/lib/Analysis/NaCl/PNaClABIVer...
lib/Analysis/NaCl/PNaClABIVerifyModule.cpp:127: for (unsigned i = 0; i <
AcceptableSizes.size(); ++i)
Would it be cleaner to iterate over it (it has iterator support)?

https://codereview.chromium.org/14914011/diff/1/lib/Analysis/NaCl/PNaClABIVer...
lib/Analysis/NaCl/PNaClABIVerifyModule.cpp:133: static bool
FunctionHasIntArg1(const Function* F,
The name is not entirely clear... also you may be over-generalizing here just
for one case ;-)

https://codereview.chromium.org/14914011/diff/1/lib/Analysis/NaCl/PNaClABIVer...
lib/Analysis/NaCl/PNaClABIVerifyModule.cpp:155: case Intrinsic::bswap: {
I think this would be clearer in a method - isWhitelistedBswap. Also mention
that it whitelists certain overloads in a comment. The method can then have
accepted_sizes as static const.

[jvoung (off chromium), 2013-05-16 16:45:00]

https://codereview.chromium.org/14914011/diff/1/lib/Analysis/NaCl/PNaClABIVer...
File lib/Analysis/NaCl/PNaClABIVerifyModule.cpp (right):

https://codereview.chromium.org/14914011/diff/1/lib/Analysis/NaCl/PNaClABIVer...
lib/Analysis/NaCl/PNaClABIVerifyModule.cpp:127: for (unsigned i = 0; i <
AcceptableSizes.size(); ++i)
On 2013/05/15 23:34:54, eliben wrote:
> Would it be cleaner to iterate over it (it has iterator support)?

Done.

https://codereview.chromium.org/14914011/diff/1/lib/Analysis/NaCl/PNaClABIVer...
lib/Analysis/NaCl/PNaClABIVerifyModule.cpp:133: static bool
FunctionHasIntArg1(const Function* F,
On 2013/05/15 23:34:54, eliben wrote:
> The name is not entirely clear... also you may be over-generalizing here just
> for one case ;-)


Well, I assume I'll be checking parameter types for some of the other overloaded
intrinsics.

For now, just keeping the TypeAcceptable() function, and replacing this one with
isWhitelistedBswap().

https://codereview.chromium.org/14914011/diff/1/lib/Analysis/NaCl/PNaClABIVer...
lib/Analysis/NaCl/PNaClABIVerifyModule.cpp:155: case Intrinsic::bswap: {
On 2013/05/15 23:34:54, eliben wrote:
> I think this would be clearer in a method - isWhitelistedBswap. Also mention
> that it whitelists certain overloads in a comment. The method can then have
> accepted_sizes as static const.

Done.

[jvoung (off chromium), 2013-05-21 23:32:36]

ping

[Derek Schuff, 2013-05-22 15:58:47]

lgtm

[jvoung (off chromium), 2013-05-22 21:34:30]

Committed patchset #2 manually as r5019000 (presubmit successful).