align() not respected for local variable declarations

[redstar]

This application

// LDC - the LLVM D compiler (0.16.0-beta2):
//  based on DMD v2.067.1 and LLVM 3.7.0
//  Default target: x86_64-pc-windows-msvc
//
// ldc2 main.d -w -g -unittest -m64 -oq

void main(string[] args)
{
}

align(16) struct some_buf
{
align(16):
    long[32] data;
}

int somefun(ref some_buf) { return 0; }

unittest
{
    import std.stdio;
    some_buf buf;
    writeln(&buf);
    assert((cast(size_t)&buf & cast(size_t)0b1111) == 0);
    void foo()
    {
        writeln("foo");
        somefun(buf);
    }
    foo();
}

asserts because variable some_buf is only align(8), not align(16).

[kinke]

Seems to work for struct fields only:

struct Struct { align(16) int[2] a; }

=> struct is padded to a size of 16 bytes, and gets properly aligned on stack:

%align.Struct = type { [2 x i32], [8 x i8] }
%s = alloca %align.Struct, align 16             ; [#uses = 1, size/byte = 16]

Struct itself:

align(16) struct Struct { int[2] a; }

=> struct is padded to 16 bytes too, but alignment = 4:

%s = alloca %align.Struct, align 4              ; [#uses = 1, size/byte = 16]

A local variable align(16) int x; is falsely 4-bytes aligned too.

Edit:
The some_buf struct above works for me on Win64 with merge-2.068.

Edit 2:
The some_buf struct itself works, but the problem in that unittest is nested non-static foo(), which leads to buf being stored inside a nested context struct, where the alignment is apparently lost.

[kinke]

I'll work on the nested context issue tomorrow. I'll probably come up with an LLStructBuilder, which allows for aligned fields by inserting padding if need be and providing a field index -> real GEP index mapping (need to skip over padding i8-arrays) as well as a required overall struct alignment (the max of all fields' individual alignments if I'm right) when later allocating such a struct on the stack. Some of this (aligned fields) is already implemented by the RTTIBuilder class.

[kinke]

The issue wrt. the align attribute for structs can be mitigated by using Type::alignment() (if != STRUCTALIGN_DEFAULT) instead of Type::alignsize() in DtoAlloca().

[kinke]

The original test case is fixed by #1159. What still needs to be fixed is the alignment of local variables.

[dnadlinger]

@kinke: Your testcase at https://github.com/ldc-developers/ldc/blob/61dd702b4909f08f4386862707b5b05bc894bfef/tests/ir/align.d already seems to check for at least some cases of local variables, though?

[kinke]

That's true, but that's only because of the type's explicit/implicit alignment. Explicit alignments for locals, i.e., something like align(64) Outer outer; does not work (falling back to the type's 32-bytes). It's not set in VarDeclaration::alignment.

[dnadlinger]

Ah, I see, thanks.

[Temtaime]

http://goo.gl/ikkwvc
As one can see, LDC also ignores align attribute and generates movups instuction.

[dnadlinger]

@Temtaime: Could you please open separate enhancement requests for missed optimization opportunities? This bug report is about alignment not respected for variables, which is incorrect code, as opposed to just suboptimal code.

[kinke]

Okay, let's close this, as all of this + some more is checked by https://github.com/ldc-developers/ldc/blob/master/tests/ir/align.d now. Exception: aligned captured variables in nested context structs, which is fixed but not tested yet.