[PATCH] [AArch64] Change int64_t from 'long long int' to 'long int' for AArch64 target.

[Ana Pazos]

Hi folks,

I created a test file where I include arm_neon.h.

I am using Linaro GCC 4.8 (calling the installation dir $GCC48). And it
fails to compile my test with errors like:

	typedef __attribute__((neon_vector_type(1)))  int64_t int64x1_t;

I invoke clang setting -target, --sysroot, -gcc-toolchain, mfpu=neon
variables:

	clang -mfpu=neon --target=aarch64-linux-gnu
-sysroot=$$GCC48/aarch64-linux-gnu/libc --gcc-toolchain=$GCC48 test.c

The problem is the word size defined as 64 bits in this gcc header file:
	$GCC48/aarch64-linux-gnu/libc/usr/include/bits/wordsize.h

It should be 32 bits.

It seems this commit from Jiangning works around the in GCC sysroot issue:
	3dc9e80234b0112a0463feaf65049bcd858d1d36 (For AArch64, support
builtin neon vector type with 'long' as base element type) 

But it looks like GCC should be fixed instead.

What do you think?

Ana.


-----Original Message-----
From: cfe-commits-bounces at cs.uiuc.edu
[mailto:cfe-commits-bounces at cs.uiuc.edu] On Behalf Of Tim Northover
Sent: Thursday, February 20, 2014 5:49 AM
To: reviews+D2845+public+5d5f24294ac1b103 at llvm-reviews.chandlerc.com
Cc: llvm-commits; joerg at netbsd.org; cfe-commits at cs.uiuc.edu
Subject: Re: [PATCH] [AArch64] Change int64_t from 'long long int' to 'long
int' for AArch64 target.

Hi Kevin,

I think this patch will break 32-bit ARM and 64-bit Darwin, both of which
define int64_t to be "long long" for various reasons (32-bit ARM because
"long" is 32-bits, Darwin because Just Because. ;-)).

ItaniumMangle should be fine: that mangling is only used on AArch64
non-Darwin so globally changing it is fine. Targets.cpp looks fine to me as
well.

For SemaChecking.cpp, the getNeonEltType is shared among all
implementations; perhaps give it another bool parameter and rename the
existing one? IsPolyUnsigned & IsInt64Long for example.

NeonEmitter.cpp is a bigger problem, since there doesn't appear to be an
"int64_t" type string available. It seems reasonably easy to add one
(lib/AST/ASTContext.cpp -- DecodeTypeFromStr and a comment in Builtins.def),
but I'm not sure if there's a better solution (we're running out of
letters!)

There are also a couple of bugs on the host side:

-        qmask |= 1ULL << GetNeonEnum(Proto, TypeVec[ti]);
+        qmask |= 1UL << GetNeonEnum(Proto, TypeVec[ti]);

qmask and mask are Clang-internal 64-bit variables. This change will
truncate various lines in arm_neon.inc (#included into
lib/Sema/SemaChecking.cpp) when Clang is built on platforms with a 32-bit
long.

The same goes for the change to the "case".

Cheers.

Tim.
_______________________________________________
cfe-commits mailing list
cfe-commits at cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits