[PATCH] D80934: [AIX] Update data directives for AIX assembly

[David Tenty via Phabricator via llvm-commits]

daltenty marked 2 inline comments as done.
daltenty added inline comments.


================
Comment at: llvm/lib/MC/MCAsmInfoXCOFF.cpp:33
+  // The standard AIX assembly directives auto-align, so they are not usable.
+  Data16bitsDirective = "\t.vbyte\t2, ";
+  Data32bitsDirective = "\t.vbyte\t4, ";
----------------
Xiangling_L wrote:
> Just a question here.
> I checked the XL behavior, in the following case:
> 
> ```
>   struct {
>     int i;
>     double n;
>   } a[] = {
>            {9, 1.5},
>   };
> 
> ```
> 
> test.s:
> 
> ```
>         .csect  a{RW}, 3
>         .long   0x00000009              # "\0\0\0\t"
>         .long   0x3ff80000              # "?\370\0\0"
>         .long   0x00000000              # "\0\0\0\0"
> ```
> 
> Under 32-bit mode, XL split a double to two `.long`. So I am wondering can we do the same in LLVM?
Currently we cannot (we'll split it into two `.vbyte 4,` the way gcc does), as `.long` has implicit alignment behaviour (i.e. what we are trying to avoid in this patch) and at the point of value emission we don't have enough context to tell if it's safe to use. 

AFAIK the streamer is expecting alignment constraints to already be taken care of explicitly by emission of `.align`,etc. by the time we get to emitting values, so we can't use any of the implicitly aligned directives.


================
Comment at: llvm/test/CodeGen/PowerPC/aix-xcoff-data.ll:29
+%struct.anon = type <{ i32, double }>
+ at astruct = global [1 x %struct.anon] [%struct.anon <{ i32 1, double 7.000000e+00 }>], align 1
+
----------------
Xiangling_L wrote:
> I am wondering do you make it align to 1 due to any special reason? If not, according to AIX power alignment rule, this struct is supposed to be aligned to 4.
> 
> And also, we may want to add a more interesting case where the double is the first member of the struct, which is 8-byte aligned.
> e.g.
> 
> ```
> %struct.anon = type <{ double, i32 }>
> @astruct = global [1 x %struct.anon] [%struct.anon <{ double 7.000000e+00 , i32 1}>], align 8
> ```
> 
> By doing that, we can see how the padding is added using `vbyte`.
> I am wondering do you make it align to 1 due to any special reason? If not, according to AIX power alignment rule, this struct is supposed to be aligned to 4.

IIUC, we needn't be concerned with this adhering to AIX power alignment rules, we can validly generate this packed form out of clang with `__attribute__ ((packed))`. I had used the packed form since we were mainly interested in observing the effect of data directives (and thus any implicit alignment) used to write out the members, rather than anything from the ABI.

> And also, we may want to add a more interesting case where the double is the first member of the struct, which is 8-byte aligned.
>By doing that, we can see how the padding is added using vbyte.

The padding is actually emitted using `.space` directives, so I'm not sure if it buys us much in the context of this change, but I'll add this as a non-packed case for completeness.



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D80934/new/

https://reviews.llvm.org/D80934