Skip to content

Commit

Permalink
[CIR][CIRGen] Fix zero-offset global view access
Browse files Browse the repository at this point in the history
Instead of ignoring global view access with zero offset, we should
properly dereference the global type to prevent type-matching errors.

This patch also fixes other two issues:

 - An extra index was wrongly added to the global view access. E.g.
   for an access like `a[0]` would generate a GV with 2 zero indexes.
   The indexes, however, do not take the pointer wrapping the global
   type into account.
 - When assigning the address of a complete type to an incomplete one
   the complete type would override the incomplete destination type,
   causing inconsistencies during CodeGen. For example, given `int a[3];`
   and `int (*ptr_a)[] = &a;`, despite `ptr_a`, in CIR it would be
   considered complete.

Fixes #329
  • Loading branch information
sitio-couto authored and lanza committed Apr 29, 2024
1 parent 90a1894 commit 6074676
Show file tree
Hide file tree
Showing 3 changed files with 56 additions and 14 deletions.
21 changes: 13 additions & 8 deletions clang/lib/CIR/CodeGen/CIRGenExprConst.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1066,22 +1066,27 @@ class ConstantLValueEmitter
Indices.push_back(Attr);
}

if (Indices.empty())
return {};
return CGM.getBuilder().getArrayAttr(Indices);
}

// TODO(cir): create a proper interface to absctract CIR constant values.

/// Apply the value offset to the given constant.
ConstantLValue applyOffset(ConstantLValue &C) {
if (!hasNonZeroOffset())
return C;

if (auto Attr = C.Value.dyn_cast<mlir::Attribute>()) {
auto GV = cast<mlir::cir::GlobalViewAttr>(Attr);
assert(!GV.getIndices());

return mlir::cir::GlobalViewAttr::get(
GV.getType(), GV.getSymbol(), getOffset(GV.getType()));
// Handle attribute constant LValues.
if (auto Attr =
C.Value.dyn_cast<mlir::Attribute>()) {
if (auto GV = Attr.dyn_cast<mlir::cir::GlobalViewAttr>()) {
auto baseTy = GV.getType().cast<mlir::cir::PointerType>().getPointee();
auto destTy = CGM.getTypes().convertTypeForMem(DestType);
assert(!GV.getIndices() && "Global view is already indexed");
return mlir::cir::GlobalViewAttr::get(destTy, GV.getSymbol(),
getOffset(baseTy));
}
llvm_unreachable("Unsupported attribute type to offset");
}

// TODO(cir): use ptr_stride, or something...
Expand Down
20 changes: 16 additions & 4 deletions clang/test/CIR/CodeGen/array.c
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,25 @@ struct S {
// CHECK: cir.global external @arr = #cir.const_array<[#cir.const_struct<{#cir.int<1> : !s32i}> : !ty_22S22, #cir.zero : !ty_22S22, #cir.zero : !ty_22S22]> : !cir.array<!ty_22S22 x 3>

int a[4];
int (*ptr_a)[] = &a;
// CHECK: cir.global external @a = #cir.zero : !cir.array<!s32i x 4>
// CHECK: cir.global external @ptr_a = #cir.global_view<@a> : !cir.ptr<!cir.array<!s32i x 4>>

// Should create a pointer to a complete array.
int (*complete_ptr_a)[4] = &a;
// CHECK: cir.global external @complete_ptr_a = #cir.global_view<@a> : !cir.ptr<!cir.array<!s32i x 4>>

// Should create a pointer to an incomplete array.
int (*incomplete_ptr_a)[] = &a;
// CHECK: cir.global external @incomplete_ptr_a = #cir.global_view<@a> : !cir.ptr<!cir.array<!s32i x 0>>

// Should access incomplete array if external.
extern int foo[];
// CHECK: cir.global "private" external @foo : !cir.array<!s32i x 0>

void useFoo(int i) {
foo[i] = 42;
}
}
// CHECK: @useFoo
// CHECK: %[[#V2:]] = cir.get_global @foo : cir.ptr <!cir.array<!s32i x 0>>
// CHECK: %[[#V3:]] = cir.load %{{.+}} : cir.ptr <!s32i>, !s32i
// CHECK: %[[#V4:]] = cir.cast(array_to_ptrdecay, %[[#V2]] : !cir.ptr<!cir.array<!s32i x 0>>), !cir.ptr<!s32i>
// CHECK: %[[#V5:]] = cir.ptr_stride(%[[#V4]] : !cir.ptr<!s32i>, %[[#V3]] : !s32i), !cir.ptr<!s32i>
// CHECK: cir.store %{{.+}}, %[[#V5]] : !s32i, cir.ptr <!s32i>
29 changes: 27 additions & 2 deletions clang/test/CIR/CodeGen/globals.c
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@
// bit different from the C++ version. This test ensures that these differences
// are accounted for.

// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-cir %s -o - | FileCheck %s
// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-cir %s -o %t.cir
// RUN: FileCheck --input-file=%t.cir %s

char string[] = "whatnow";
// CHECK: cir.global external @string = #cir.const_array<"whatnow\00" : !cir.array<!s8i x 8>> : !cir.array<!s8i x 8>
Expand Down Expand Up @@ -48,7 +49,31 @@ struct {
// CHECK: cir.global external @nestedStringPtr = #cir.const_struct<{#cir.global_view<@".str"> : !cir.ptr<!s8i>}>

int *globalPtr = &nestedString.y[1];
// CHECK: cir.global external @globalPtr = #cir.global_view<@nestedString, [0 : i32, 1 : i32, 1 : i32]>
// CHECK: cir.global external @globalPtr = #cir.global_view<@nestedString, [1 : i32, 1 : i32]> : !cir.ptr<!s32i>

const int i = 12;
int i2 = i;
struct { int i; } i3 = {i};
// CHECK: cir.global external @i2 = #cir.int<12> : !s32i
// CHECK: cir.global external @i3 = #cir.const_struct<{#cir.int<12> : !s32i}> : !ty_22anon2E722

int a[10][10][10];
int *a2 = &a[3][0][8];
struct { int *p; } a3 = {&a[3][0][8]};
// CHECK: cir.global external @a2 = #cir.global_view<@a, [3 : i32, 0 : i32, 8 : i32]> : !cir.ptr<!s32i>
// CHECK: cir.global external @a3 = #cir.const_struct<{#cir.global_view<@a, [3 : i32, 0 : i32, 8 : i32]> : !cir.ptr<!s32i>}> : !ty_22anon2E922

int p[10];
int *p1 = &p[0];
struct { int *x; } p2 = {&p[0]};
// CHECK: cir.global external @p1 = #cir.global_view<@p> : !cir.ptr<!s32i>
// CHECK: cir.global external @p2 = #cir.const_struct<{#cir.global_view<@p> : !cir.ptr<!s32i>}> : !ty_22anon2E1122

int q[10];
int *q1 = q;
struct { int *x; } q2 = {q};
// CHECK: cir.global external @q1 = #cir.global_view<@q> : !cir.ptr<!s32i>
// CHECK: cir.global external @q2 = #cir.const_struct<{#cir.global_view<@q> : !cir.ptr<!s32i>}> : !ty_22anon2E1322

// TODO: test tentatives with internal linkage.

Expand Down

0 comments on commit 6074676

Please sign in to comment.