PNaCl: Handle nested structure types in -expand-struct-regs.

test/Transforms/NaCl/expand-struct-regs.ll

 ; RUN: opt %s -expand-struct-regs -S | FileCheck %s
 ; RUN: opt %s -expand-struct-regs -S | FileCheck %s -check-prefix=CLEANUP
 
 ; These two instructions should not appear in the output:
 ; CLEANUP-NOT: extractvalue
 ; CLEANUP-NOT: insertvalue
 
 %struct = type { i8, i32 }
 
 
@@ skipping 106 matching lines @@
 ; CHECK-NEXT: store i32 200, i32* %out1
 ; CHECK-NEXT: ret void
 
 
 define i32 @extract_from_constant() {
   %ev = extractvalue %struct { i8 99, i32 888 }, 1
   ret i32 %ev
 }
 ; CHECK: define i32 @extract_from_constant() {
 ; CHECK-NEXT: ret i32 888
+
+define void @nested_structs() {
+  %a1 = alloca i64
+  %a2 = alloca i32
+  %a3 = alloca { { i32, i64 } }
+  %a = insertvalue { i32, i64 } undef, i32 5, 0
+  %b = insertvalue { i32, i64 } %a, i64 6, 1
+  %c = insertvalue { { i32, i64 } } undef, { i32, i64 } %b, 0
+  %d = insertvalue { { { i32, i64 } }, i64 } undef, { { i32, i64 } } %c, 0
+  %e = insertvalue { { { i32, i64 } }, i64 } undef, { i32, i64 } %b, 0, 0
+
+  %f = extractvalue { { { i32, i64 } }, i64 } %d, 0, 0, 1
+  %g = extractvalue { { { i32, i64 } }, i64 } %e, 0, 0, 0
+  %h = extractvalue { { { i32, i64 } }, i64 } %e, 0
+  store i64 %f, i64* %a1
+  store i32 %g, i32* %a2
+  store { { i32, i64 } } %h, { { i32, i64 } }* %a3
+  ret void
+}
+; CHECK-LABEL: define void @nested_structs()
+; CHECK-NEXT:    %a1 = alloca i64
+; CHECK-NEXT:    %a2 = alloca i32
+; CHECK-NEXT:    %a3 = alloca { { i32, i64 } }
+; CHECK-NEXT:    store i64 6, i64* %a1
+; CHECK-NEXT:    store i32 5, i32* %a2
+; CHECK-NEXT:    %a3.index = getelementptr { { i32, i64 } }* %a3, i32 0, i32 0
+; CHECK-NEXT:    %a3.index.index = getelementptr { i32, i64 }* %a3.index, i32 0, i32 0
+; CHECK-NEXT:    store i32 5, i32* %a3.index.index, align 1
+; CHECK-NEXT:    %a3.index.index1 = getelementptr { i32, i64 }* %a3.index, i32 0, i32 1
+; CHECK-NEXT:    store i64 6, i64* %a3.index.index1, align 1
+
+define void @load_another_pass() {
+  %a = alloca { { i64 } }
+  %b = load { { i64 } }* %a
+  ret void
+}
+; CHECK-LABEL: define void @load_another_pass()
+; CHECK-NEXT:    %a = alloca { { i64 } }
+; CHECK-NEXT:    %b.index = getelementptr { { i64 } }* %a, i32 0, i32 0
+; CHECK-NEXT:    %b.field.index = getelementptr { i64 }* %b.index, i32 0, i32 0
+; CHECK-NEXT:    %b.field.field = load i64* %b.field.index, align 1

[JF, 2014/12/25 17:09:36]

Could you have a test for alignment preservation too, say with { { i8, i8, i64,
i32 }} to make sure that each field is aligned as expected (I think this should
be 8, 1, (padding), 8, 8).

Same on store.

PNaCl will later drop integer alignment (and preserve floating-point / vector)
but we want these passes to be upstreamable, so they need to be general enough.

[Richard Diamond, 2014/12/26 01:57:45]

This pass didn't preserve alignment before this PR and I think that such work
should be done in another PR.

[JF, 2014/12/26 18:14:41]

That's acceptable, though please note that open source often is about improving
more than what you came in to improve. In this case it'll require using
MathExtras.h:MinAlign and should be about 2 lines of code for load, same on
store. I don't mind another patch, but I would like at least a TODO.

[Richard Diamond, 2014/12/27 01:19:37]

Ah, I was unaware of MathExtras existence. That simplifies things.
I've added the alignment preservation.

+; CHECK-NEXT:    ret void
+
+define void @store_another_pass() {
+  %a = alloca { { i64 } }
+  store { { i64 } } undef, { { i64 } }* %a
+  ret void
+}
+; CHECK-LABEL: define void @store_another_pass()
+; CHECK-NEXT:    %a = alloca { { i64 } }
+; CHECK-NEXT:    %a.index = getelementptr { { i64 } }* %a, i32 0, i32 0
+; CHECK-NEXT:    %a.index.index = getelementptr { i64 }* %a.index, i32 0, i32 0
+; CHECK-NEXT:    store i64 undef, i64* %a.index.index, align 1
+; CHECK-NEXT:    ret void
+
+define void @select_another_pass() {
+  %a = select i1 undef, { { i64 } } undef, { { i64 } } undef
+  ret void
+}
+; CHECK-LABEL: define void @select_another_pass()
+; CHECK-NEXT:    %a.index.index = select i1 undef, i64 undef, i64 undef

[JF, 2014/12/25 17:09:36]

Could you also test with a selected value of { { i64, i64 } } and then store the
value? I want to make sure that both fields get stored.

Could you also use globals of type { { i64 } } here instead of undef? Just to
make sure parameter order is preserved into the select.

[Richard Diamond, 2014/12/26 01:57:45]

I used a local load from null (lol), so there are unique values for both value
operands.

+; CHECK-NEXT:    ret void
+
+define void @phi_another_pass() {
+entry:
+  br label %next
+
+next:
+  %a = phi { { i64 } } [ undef, %entry ]
+  ret void
+}
+; CHECK-LABEL: define void @phi_another_pass()
+; CHECK:         %a.index.index = phi i64 [ undef, %entry ]

[JF, 2014/12/25 17:09:36]

Same as select, please add a phi with { { i64, i64 } }. Could you also have two
incoming basic blocks, to ensure order is preserved?

[Richard Diamond, 2014/12/26 01:57:45]

Done.

+; CHECK-NEXT:    ret void