[cfe-commits] [PATCH] Properly factor Native Client defines to support NaCl as an OS with x86/ARM architecture

[Derek Schuff]

Thanks for the feedback. I believe I've taken care of the comments
(I'm not 100% familiar with verify tests, hopefully this is what you
meant). New patches attached.

-Derek

On Tue, Oct 9, 2012 at 3:17 PM, Eli Friedman <eli.friedman at gmail.com> wrote:
> On Thu, Sep 27, 2012 at 11:24 AM, Derek Schuff <dschuff at google.com> wrote:
>> Hi,
>> This patch refactors the NaCl/PNaCl support in clang such that the
>> defines which belong to the NaCl OS (e.g. __native_client__) come from
>> an OSTargetInfo and only those which are for Portable NaCl ( __pnacl__
>> and __le32__) come from the PNaClTargetInfo.
>> It thus allows the compiler to support e.g. x86_64-nacl and arm-nacl
>> targets (as well as le32-nacl). It would be a very small change except
>> that the x86_64 nacl ABI is X32-like with 32-bit pointers, and does
>> not use 80-bit float types. So, some small changes to e.g.
>> X86_64ABIInfo.
>>
>> This patch is split into 2 parts. The first is a change to
>> X86_64ABIInfo to support x86-64 targets with 32 bit pointers. The
>> approach is similar to the change proposed by the X32 team[1]
>> (although it is slightly more general in its naming scheme and does
>> not have X32-specific bits).
>>
>> The second change actually makes the NaCl specific changes. There is
>> one small intrusion into X86_64ABIInfo::classify to disable the 80-bit
>> FP types (suggestions on that are welcome)
>>
>> Patches are attached or may be viewed at
>> http://codereview.chromium.org/10982073 and
>> http://codereview.chromium.org/10986071
>>
>> Please take a look, thanks.
>> -Derek
>
> test/CodeGen/x86_64-arguments-nacl.c
> @@ -0,0 +1,347 @@
> +// RUN: %clang_cc1 -triple x86_64-unknown-nacl -emit-llvm -o - %s| FileCheck %s
> +// RUN: %clang_cc1 -triple x86_64-unknown-nacl -emit-llvm -o - %s
> -target-feature +avx | FileCheck %s -check-prefix=AVX
> +#include <stdarg.h>
>
> Copy-pasting the entire test is excessive; please try to test just the
> relevant pieces.
>
> --- /dev/null
> +++ b/test/Driver/x86_64-unknown-nacl.cpp
> @@ -0,0 +1,141 @@
> +// RUN: %clang -target x86_64-unknown-nacl -ccc-clang-archs x86_64
> -ccc-echo %s -emit-llvm-only -c 2>&1 | FileCheck %s -check-prefix=ECHO
> +// RUN: %clang -target x86_64-unknown-nacl -ccc-clang-archs x86_64 %s
> -emit-llvm -S -c -o - | FileCheck %s
> +// RUN: %clang -target x86_64-unknown-nacl -ccc-clang-archs x86_64 %s
> -emit-llvm -S -c -pthread -o - | FileCheck %s -check-prefix=THREADS
>
> Please separate out the tests for defines and tests for
> target-specific properties which could be written as a -verify test.
>
> --- a/lib/CodeGen/TargetInfo.cpp
> +++ b/lib/CodeGen/TargetInfo.cpp
> @@ -1114,10 +1114,13 @@ class X86_64ABIInfo : public ABIInfo {
>    }
>
>    bool HasAVX;
> +  bool Has64BitPointers;
>
> A comment about the circumstances where pointers aren't 64 bits on
> x86-64 would be appropriate here.
>
> -Eli
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 32bit-ptr-2.diff
Type: application/octet-stream
Size: 5194 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20121010/31fd8c72/attachment.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: nacl-abi-2.diff
Type: application/octet-stream
Size: 11484 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20121010/31fd8c72/attachment-0001.obj>