[PATCH] Synchronize the NaCl DataLayout string with the one in clang

[Rafael Espíndola]

On 18 December 2013 16:22, Derek Schuff <dschuff at google.com> wrote:
>
>   Thanks for looking at this.
>   Just to refresh my memory, the preferred alignment defaults to the ABI alignment, so e.g. i64:64:64 is equivalent to i64:64, correct?

Correct.

>
> ================
> Comment at: lib/Target/ARM/ARMTargetMachine.cpp:93
> @@ +92,3 @@
> +  if (ST.isTargetNaCl())
> +    Ret += "-v128:32";
> +  else if (ST.isAPCS_ABI())
> ----------------
> NaCl uses AAPCS VFP conventions, with a stack alignment of 16 bytes. Would it make more sense just to set that in ARMSubtarget.cpp instead of special casing here?

Maybe, but that is not what clang currently sets. In particular, it
sets no stack alignment. This patch was just making llvm produce the
same strings as clang.

If you want we can try to make llvm produce the correct string first
and then update clang to match.

It also looks like NaCl already uses triples with an eabi environment,
so we might not need to update ARMSubtarget.cpp. Just set a more
aligned stack in ARMTargetMachine.cpp.

I will upload a patch along these lines to a different review.

>
> ================
> Comment at: lib/Target/X86/X86TargetMachine.cpp:43
> @@ -43,1 +42,3 @@
> +  if (ST.is64Bit() || ST.isTargetCygMing() || ST.isTargetWindows() ||
> +      ST.isTargetNaCl())
>      Ret += "-i64:64";
> ----------------
> I'm a little unclear about this; does this mean 64-bit targets have i64 but not f64?

No, they just use the default (f64:64).

> NaCl does use f64:64 and i64:64 for both 32 and 64 bits, but I don't see where else but here that f64 is even set.
>
> ================
> Comment at: lib/Target/X86/X86TargetMachine.cpp:56
> @@ -52,16 +55,3 @@
>
> -  // Objects on the stack ore aligned to 64 bits.
> -  if (ST.is64Bit())
> -    Ret += "-s:64";
> -
> -  // The registers can hold 8, 16, 32 or, in x86-64, 64 bits.
> -  if (ST.is64Bit())
> -    Ret += "-n8:16:32:64";
> -  else
> -    Ret += "-n8:16:32";
> -
> -  // The stack is aligned to 32 bits on some ABIs and 128 bits on others.
> -  if (!ST.is64Bit() && (ST.isTargetCygMing() || ST.isTargetWindows()))
> -    Ret += "-S32";
> -  else
> -    Ret += "-S128";
> +  if (ST.isTargetNaCl())
> +    Ret += "-v128:32";
> ----------------
> This can be removed. Clang seems to have it it upstream, but that's wrong. vectors are never generated for le32, and for i686-nacl and x86_64-nacl they have the same alignment as linux.
>
> ================
> Comment at: lib/Target/X86/X86TargetMachine.cpp:59
> @@ +58,3 @@
> +
> +  if (!ST.isTargetNaCl()) {
> +    // Objects on the stack ore aligned to 64 bits.
> ----------------
> I think this condition can move to just the stack alignment, as we use s32-n8:16:32-S128 for i686-nacl and s:64-n8:16:32:64-S128 for x86_64-nacl

OK.

Cheers,
Rafael