enum / tag alignment is always 1 right now

[nikomatsakis]

Right now, we do not compute the correct alignment for enum types. Effectively we treat them as [u8] at all times. This causes a variety of problems. Mostly it causes poor interaction with shape code and C code, which assumes that we do alignment correctly. I think there is probably an issue for this but I haven't found one that specifically describes it.

Some symptoms:

• 1535: Replacing a type like int with enum foo { foo(int) } is not equivalent. It will cause shape code to misbehave when doing comparisons and the like because shape code will try to align to a word boundary, but the data will be stored with no alignment at all. (For example, I tried to replace ty::t with such a type)

• Converting tag discriminants from int to i32 does not work because the payloads are no longer properly aligned on 64-bit targets. Basically, it works now by accident because int is 64-bits and we don't have any payloads of greater than 8-byte alignment.

[nikomatsakis]

Some notes: this is a pain to fix right now due to the possibility of generic tags. Monomorphizing would help.

[nikomatsakis]

I put this on a 0.3 milestone. It should now be feasible thanks to monomorphization.

[brson]

After we convert enums to LLVM named structs we may be able to specify their alignment.

[brson]

I don't see a way to tell LLVM the alignment of a type.

[nikomatsakis]

There must be a way, since gcc can do it (with attributes).

[nikomatsakis]

Hmm, you may be right. I looked at how clang handled this C file:

struct registers_t {
    char data[22];
} __attribute__((aligned(16)));

struct foo { struct registers_t regs; };

void bar(struct foo *s) {} 

int main() {}

and it was not illuminating:

; ModuleID = 'align.c'
target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32:64"
target triple = "x86_64-apple-macosx10.7.3"

%struct.foo = type { %struct.registers_t }
%struct.registers_t = type { [22 x i8], [10 x i8] }

define void @bar(%struct.foo* %s) nounwind ssp {
  %1 = alloca %struct.foo*, align 8
  store %struct.foo* %s, %struct.foo** %1, align 8
  ret void
}

define i32 @main() nounwind ssp {
  ret i32 0
}

[nikomatsakis]

It appears that clang just inserts the necessary padding and so forth itself. How disappointing.

[brson]

OK, I have the fix in the works and need more test cases. I've found the four tag-align tests in run-pass.

[brson]

I ran into an obstacle on 32-bit x86 #2303.

[nikomatsakis]

A good test would be something like:

enum g = option<u64>;
type r = {f: u8, g: g};
fn main() {
    let x = {f: 1_u8, g: g(some(22_u64))};
    let y = {f: 1_u8, g: g(some(22_u64))};
   assert x == y;
}

This currently segfaults for me.

[luqmana]

This should be closed now as it seems to have been fixed.

[jld]

Univariants are correct now, and everything agrees with everything else (I think) as far as how enums are represented, but that representation is not actually correct: non-univariant enum bodies are aligned only as much as the discriminant. But I'm not sure if this issue was meant to cover that.

[nikomatsakis]

I think we ought to align non-univariant enums the same way that a C struct like so would be aligned:

struct {
    unsigned discriminant; // or whatever numeric type we use
    union {
        struct Variant1 { /* args of variant 1 */ };
        ...
        struct Variant1 { /* args of variant N */ };
    };
};

That's what this issue was meant to cover. Based on @jld's comment I take it that this is not yet true in all cases, though because we use a uint discriminant (64-bit on 64-bit systems) it often works in practice.

[nikomatsakis]

(Or rather, this issue was meant to cover both the univariant case (which sounds like it is fixed properly) and the multivariant case (not entirely))

[jld]

So, for the multivariant case, I was thinking more like this:

union {
   struct { some_int_t discr; /* args of variant 1 */ } v1;
   /* ... */
   struct { some_int_t discr; /* args of variant N */ } vN;
};

This means that cases of less-than-maximal alignment can possibly start earlier, which could save space if they're the longest. Also, it makes things simpler to implement.