Skip to content

Commit cf39106

Browse files
gitoleglanza
authored andcommitted
[CIR][CodeGen] Updates GlobalViewAttr's indices computation for the union type (llvm#1584)
This PR introduces a new attribute for the explicit byte offset for `GlobalViewAttr`. It's a proposal, so feel free to reject it once it's not good from your point of view. The problem is (as usually) with globals and unions: looks like we can not really use indexes in the `GlobalView` to address an array of unions. For example, the next program prints `4` now, but it should be `42`: ``` typedef struct { long s0; int s1; } S; typedef union { int f0; S f1; } U; static U g1[3] = {{42},{42},{42}}; int* g2 = &g1[1].f1.s1; int main() { (*g2) = 4; printf("%d\n", g1[1].f0); return0; } ``` The problem is that we compute wrong indices in `CIRGenBuilder::computeGlobalViewIndicesFromFlatOffset`. Maybe it can be even fixed in this case, but I have a feeling that the fix would be a bit fragile. So instead of trying to support indexes for the array of unions I suggest to use the offset explicitly. From the implementation point of view there are some changes in `CIRGenBuilder ` - but nothing really new is in there - I just did not want to introduce copy-pasta for the `isOffsetInUnion` function that is pretty the same as former `computeGlobalViewIndicesFromFlatOffset`.
1 parent 85f587b commit cf39106

File tree

2 files changed

+28
-2
lines changed

2 files changed

+28
-2
lines changed

clang/lib/CIR/CodeGen/CIRGenBuilder.cpp

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -96,15 +96,21 @@ void CIRGenBuilderTy::computeGlobalViewIndicesFromFlatOffset(
9696
unsigned AlignMask = Layout.getABITypeAlign(Elts[I]).value() - 1;
9797
if (RecordTy.getPacked())
9898
AlignMask = 0;
99-
Pos = (Pos + AlignMask) & ~AlignMask;
99+
// Union's fields have the same offset, so no need to change Pos here,
100+
// we just need to find EltSize that is greater then the required offset.
101+
// The same is true for the similar union type check below
102+
if (!RecordTy.isUnion())
103+
Pos = (Pos + AlignMask) & ~AlignMask;
100104
assert(Offset >= 0);
101105
if (Offset < Pos + EltSize) {
102106
Indices.push_back(I);
103107
SubType = Elts[I];
104108
Offset -= Pos;
105109
break;
106110
}
107-
Pos += EltSize;
111+
// No need to update Pos here, see the comment above.
112+
if (!RecordTy.isUnion())
113+
Pos += EltSize;
108114
}
109115
} else {
110116
llvm_unreachable("unexpected type");

clang/test/CIR/CodeGen/union-array.c

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ static U1 g = {5};
2626
// LLVM: @g = internal global { i32 } { i32 5 }
2727
// FIXME: LLVM output should be: @g = internal global %union.U { i32 5 }
2828

29+
// LLVM: @g2 = global ptr getelementptr inbounds nuw (i8, ptr @g1, i64 24)
2930

3031
void foo() { U arr[2] = {{.b = {1, 2}}, {.a = {1}}}; }
3132

@@ -37,3 +38,22 @@ void bar(void) {
3738
}
3839
// CIR: cir.global "private" internal dsolocal @g = #cir.const_record<{#cir.int<5> : !s32i}> : !rec_anon_struct
3940
// CIR: cir.const #cir.const_array<[#cir.global_view<@g> : !cir.ptr<!s32i>, #cir.global_view<@g> : !cir.ptr<!s32i>]> : !cir.array<!cir.ptr<!s32i> x 2>
41+
42+
typedef struct {
43+
long s0;
44+
int s1;
45+
} S_3;
46+
47+
typedef union {
48+
int f0;
49+
S_3 f1;
50+
} U2;
51+
52+
53+
static U2 g1[3] = {{0x42},{0x42},{0x42}};
54+
int* g2 = &g1[1].f1.s1;
55+
// CIR: cir.global external @g2 = #cir.global_view<@g1, [1, 1, 4]> : !cir.ptr<!s32i>
56+
57+
void baz(void) {
58+
(*g2) = 4;
59+
}

0 commit comments

Comments
 (0)