llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang
Author: Adam Smith (adams381)
<details>
<summary>Changes</summary>
An enum that is still incomplete has no CIR type of its own to complete later
the way a record does, so it converts to a guessed `!u32i`.
`updateCompletedType` asserted that a definition could never disagree with that
guess. It can:
```c
extern enum E v;
void touch(void) { v; }
enum E { A = 0x100000000 };
unsigned long read(void) { return v; }
```
The definition makes `E` 64 bits and the assert fires. With assertions off the
guess survives instead, and `read` loads 32 bits of a 64-bit object and
zero-extends, where classic CodeGen loads 64.
Flushing the type cache when the definition contradicts the guess removes both
the abort and the truncation. Classic tests whether the completed type is
`i32`, which does not carry over because `!s32i` and `!u32i` are distinct CIR
types, so the comparison here is against the cached entry.
A signature converted while the enum was incomplete keeps the guess, so a
declaration can name a wrong width where classic leaves it unprototyped. No
emitted code reads that signature: call sites bitcast the callee to the
definition's type, and a definition of the function gets a fresh one. Removing
it needs CIR to reconcile placeholder function types at every conversion
boundary, since CIR function pointers are typed where LLVM's are opaque.
---
Full diff: https://github.com/llvm/llvm-project/pull/219564.diff
2 Files Affected:
- (modified) clang/lib/CIR/CodeGen/CIRGenTypes.cpp (+7-11)
- (modified) clang/test/CIR/CodeGen/forward-enum.c (+105-4)
``````````diff
diff --git a/clang/lib/CIR/CodeGen/CIRGenTypes.cpp
b/clang/lib/CIR/CodeGen/CIRGenTypes.cpp
index f1834d8fc7f37..82c69ad525674 100644
--- a/clang/lib/CIR/CodeGen/CIRGenTypes.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenTypes.cpp
@@ -818,17 +818,13 @@ void CIRGenTypes::updateCompletedType(const TagDecl *td) {
// If this is an enum being completed, then we flush all non-struct types
// from the cache. This allows function types and other things that may be
// derived from the enum to be recomputed.
- if ([[maybe_unused]] const auto *ed = dyn_cast<EnumDecl>(td)) {
- // Classic codegen clears the type cache if it contains an entry for this
- // enum type that doesn't use i32 as the underlying type, but I can't find
- // a test case that meets that condition. C++ doesn't allow forward
- // declaration of enums, and C doesn't allow an incomplete forward
- // declaration with a non-default type.
- assert(
- !typeCache.count(
- ed->getASTContext().getCanonicalTagType(ed)->getTypePtr()) ||
- (convertType(ed->getIntegerType()) ==
-
typeCache[ed->getASTContext().getCanonicalTagType(ed)->getTypePtr()]));
+ if (const auto *ed = dyn_cast<EnumDecl>(td)) {
+ const clang::Type *key = astContext.getCanonicalTagType(ed)->getTypePtr();
+ // Converting the enum before it was complete cached a guessed placeholder
+ // for its underlying type.
+ if (typeCache.count(key) &&
+ convertType(ed->getIntegerType()) != typeCache.lookup(key))
+ typeCache.clear();
// If necessary, provide the full definition of a type only used with a
// declaration so far.
assert(!cir::MissingFeatures::generateDebugInfo());
diff --git a/clang/test/CIR/CodeGen/forward-enum.c
b/clang/test/CIR/CodeGen/forward-enum.c
index cba5679485a38..6f3684321c565 100644
--- a/clang/test/CIR/CodeGen/forward-enum.c
+++ b/clang/test/CIR/CodeGen/forward-enum.c
@@ -1,9 +1,9 @@
// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fclangir -emit-cir %s -o
%t.cir
// RUN: FileCheck --check-prefix=CIR --input-file=%t.cir %s
// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fclangir -emit-llvm %s -o
%t-cir.ll
-// RUN: FileCheck --check-prefix=LLVM --input-file=%t-cir.ll %s
+// RUN: FileCheck --check-prefixes=LLVM,LLVMCIR --input-file=%t-cir.ll %s
// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm %s -o %t.ll
-// RUN: FileCheck --check-prefix=OGCG --input-file=%t.ll %s
+// RUN: FileCheck --check-prefixes=LLVM,OGCG --input-file=%t.ll %s
extern enum X x;
void f(void) {
@@ -22,5 +22,106 @@ enum X {
// LLVM: @x = external global i32
// LLVM: define {{.*}}void @f()
-// OGCG: @x = external global i32
-// OGCG: define {{.*}}void @f()
+// The same shape, but with a definition that contradicts the guess. touch_v
+// is what forces the conversion while the enum is incomplete, so uses after
+// the definition have to be given the definition's type instead.
+extern enum V v;
+void touch_v(void) { v; }
+enum V { VBig = 0x100000000 };
+unsigned long read_v(void) { return v; }
+
+// CIR: cir.func{{.*}} @touch_v()
+// CIR: cir.func{{.*}} @read_v() -> !u64i
+// CIR: %[[VP:.+]] = cir.get_global @v : !cir.ptr<!u32i>
+// CIR: %[[VCAST:.+]] = cir.cast bitcast %[[VP]] : !cir.ptr<!u32i> ->
!cir.ptr<!u64i>
+// CIR: %{{.+}} = cir.load align(8) %[[VCAST]] : !cir.ptr<!u64i>, !u64i
+
+// LLVM: define {{.*}}void @touch_v()
+// LLVM: define {{.*}}i64 @read_v()
+// LLVM: load i64, ptr @v, align 8
+
+// A fixed underlying type is not a guess, so nothing here needs invalidating.
+enum Y : long;
+extern enum Y y;
+void touch_y(void) { y; }
+enum Y : long { YOne = 1 };
+long read_y(void) { return y; }
+
+// CIR: cir.func{{.*}} @touch_y()
+// CIR: cir.get_global @y : !cir.ptr<!s64i>
+// CIR: cir.func{{.*}} @read_y() -> !s64i
+// CIR: cir.get_global @y : !cir.ptr<!s64i>
+
+// LLVM: define {{.*}}void @touch_y()
+// LLVM: load i64, ptr @y, align 8
+// LLVM: define {{.*}}i64 @read_y()
+// LLVM: load i64, ptr @y, align 8
+
+// The function pointer forces the signature to be converted while the enum is
+// incomplete, so the declaration keeps the guess. The calls are emitted after
+// the definition and use its type, bitcasting the callee to match.
+enum W;
+void takes_wider(enum W);
+void (*wider_ptr)(enum W) = takes_wider;
+enum W { WBig = 0x100000000 };
+void use_wider(void) { takes_wider(WBig); }
+void use_wider_ptr(void) { wider_ptr(WBig); }
+
+// CIR: cir.func private @takes_wider(!u32i)
+// CIR: cir.func{{.*}} @use_wider()
+// CIR: %[[WVAL:.+]] = cir.const #cir.int<4294967296> : !u64i
+// CIR: %[[WFN:.+]] = cir.get_global @takes_wider :
!cir.ptr<!cir.func<(!u32i)>>
+// CIR: %[[WCAST:.+]] = cir.cast bitcast %[[WFN]] :
!cir.ptr<!cir.func<(!u32i)>> -> !cir.ptr<!cir.func<(!u64i)>>
+// CIR: cir.call %[[WCAST]](%[[WVAL]]) : (!cir.ptr<!cir.func<(!u64i)>>,
!u64i {llvm.noundef}) -> ()
+
+// CIR: cir.func{{.*}} @use_wider_ptr()
+// CIR: %[[WPCAST:.+]] = cir.cast bitcast %{{.+}} :
!cir.ptr<!cir.ptr<!cir.func<(!u32i)>>> -> !cir.ptr<!cir.ptr<!cir.func<(!u64i)>>>
+// CIR: %[[WCALLEE:.+]] = cir.load align(8) %[[WPCAST]] :
!cir.ptr<!cir.ptr<!cir.func<(!u64i)>>>, !cir.ptr<!cir.func<(!u64i)>>
+// CIR: cir.call %[[WCALLEE]](%{{.+}}) : (!cir.ptr<!cir.func<(!u64i)>>,
!u64i {llvm.noundef}) -> ()
+
+// LLVMCIR: declare void @takes_wider(i32)
+// OGCG: declare void @takes_wider()
+// LLVM: define {{.*}}void @use_wider()
+// LLVM: call void @takes_wider(i64 noundef 4294967296)
+// LLVM: define {{.*}}void @use_wider_ptr()
+// LLVM: %[[WP:.+]] = load ptr, ptr @wider_ptr, align 8
+// LLVM: call void %[[WP]](i64 noundef 4294967296)
+
+// Same, for a definition that keeps the guess's width but not its signedness.
+enum S;
+void takes_signed(enum S);
+void (*signed_ptr)(enum S) = takes_signed;
+enum S { SNeg = -1 };
+void use_signed(void) { takes_signed(SNeg); }
+
+// CIR: cir.func private @takes_signed(!u32i)
+// CIR: cir.func{{.*}} @use_signed()
+// CIR: %[[SVAL:.+]] = cir.const #cir.int<-1> : !s32i
+// CIR: %[[SFN:.+]] = cir.get_global @takes_signed :
!cir.ptr<!cir.func<(!u32i)>>
+// CIR: %[[SCAST:.+]] = cir.cast bitcast %[[SFN]] :
!cir.ptr<!cir.func<(!u32i)>> -> !cir.ptr<!cir.func<(!s32i)>>
+// CIR: cir.call %[[SCAST]](%[[SVAL]]) : (!cir.ptr<!cir.func<(!s32i)>>,
!s32i {llvm.noundef}) -> ()
+
+// LLVMCIR: declare void @takes_signed(i32)
+// OGCG: declare void @takes_signed()
+// LLVM: define {{.*}}void @use_signed()
+// LLVM: call void @takes_signed(i32 noundef -1)
+
+// A definition emitted after the enum completes takes its parameter type from
+// the definition, so the call to it needs no bitcast. defined_ptr is again
+// what forces the early conversion.
+enum D;
+void takes_defined(enum D);
+void (*defined_ptr)(enum D) = takes_defined;
+enum D { DBig = 0x100000000 };
+unsigned long sink;
+void takes_defined(enum D d) { sink = d; }
+void use_defined(void) { takes_defined(DBig); }
+
+// CIR: cir.func{{.*}} @takes_defined(%arg0: !u64i {llvm.noundef}
+// CIR: cir.func{{.*}} @use_defined()
+// CIR: %[[DVAL:.+]] = cir.const #cir.int<4294967296> : !u64i
+// CIR: cir.call @takes_defined(%[[DVAL]]) : (!u64i {llvm.noundef}) -> ()
+
+// LLVM: define {{.*}}void @takes_defined(i64 noundef %{{.+}})
+// LLVM: define {{.*}}void @use_defined()
+// LLVM: call void @takes_defined(i64 noundef 4294967296)
``````````
</details>
https://github.com/llvm/llvm-project/pull/219564
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
