[clang] [Serialization] Read the initializer for interesting static variables before consuming it (PR #92353)
https://github.com/ChuanqiXu9 closed https://github.com/llvm/llvm-project/pull/92353 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Serialization] Read the initializer for interesting static variables before consuming it (PR #92353)
https://github.com/ChuanqiXu9 updated
https://github.com/llvm/llvm-project/pull/92353
>From 699da64855f147708f153c30177a1d02a4e014f7 Mon Sep 17 00:00:00 2001
From: Chuanqi Xu
Date: Wed, 15 May 2024 12:37:16 +0800
Subject: [PATCH 1/2] [Serialization] Read the initializer for interesting
static variables before consuming it
Close https://github.com/llvm/llvm-project/issues/91418
Since we load the variable's initializers lazily, it'd be
problematic if the initializers dependent on each other. So here we try to load
the initializers of static variables to make sure they are passed to
code generator by order. If we read any thing interesting, we would
consume that before emitting the current declaration.
---
clang/lib/Serialization/ASTReaderDecl.cpp| 29 ++-
clang/test/Modules/pr91418.cppm | 67 +
clang/test/OpenMP/nvptx_lambda_capturing.cpp | 246 +--
3 files changed, 216 insertions(+), 126 deletions(-)
create mode 100644 clang/test/Modules/pr91418.cppm
diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp
b/clang/lib/Serialization/ASTReaderDecl.cpp
index 0c647086e304a..a6254b70560c3 100644
--- a/clang/lib/Serialization/ASTReaderDecl.cpp
+++ b/clang/lib/Serialization/ASTReaderDecl.cpp
@@ -4186,12 +4186,35 @@ void ASTReader::PassInterestingDeclsToConsumer() {
GetDecl(ID);
EagerlyDeserializedDecls.clear();
- while (!PotentiallyInterestingDecls.empty()) {
-Decl *D = PotentiallyInterestingDecls.front();
-PotentiallyInterestingDecls.pop_front();
+ auto ConsumingPotentialInterestingDecls = [this]() {
+while (!PotentiallyInterestingDecls.empty()) {
+ Decl *D = PotentiallyInterestingDecls.front();
+ PotentiallyInterestingDecls.pop_front();
+ if (isConsumerInterestedIn(D))
+PassInterestingDeclToConsumer(D);
+}
+ };
+ std::deque MaybeInterestingDecls =
+ std::move(PotentiallyInterestingDecls);
+ assert(PotentiallyInterestingDecls.empty());
+ while (!MaybeInterestingDecls.empty()) {
+Decl *D = MaybeInterestingDecls.front();
+MaybeInterestingDecls.pop_front();
+// Since we load the variable's initializers lazily, it'd be problematic
+// if the initializers dependent on each other. So here we try to load the
+// initializers of static variables to make sure they are passed to code
+// generator by order. If we read anything interesting, we would consume
+// that before emitting the current declaration.
+if (auto *VD = dyn_cast(D);
+VD && VD->isFileVarDecl() && !VD->isExternallyVisible())
+ VD->getInit();
+ConsumingPotentialInterestingDecls();
if (isConsumerInterestedIn(D))
PassInterestingDeclToConsumer(D);
}
+
+ // If we add any new potential interesting decl in the last call, consume it.
+ ConsumingPotentialInterestingDecls();
}
void ASTReader::loadDeclUpdateRecords(PendingUpdateRecord ) {
diff --git a/clang/test/Modules/pr91418.cppm b/clang/test/Modules/pr91418.cppm
new file mode 100644
index 0..33fec992439d6
--- /dev/null
+++ b/clang/test/Modules/pr91418.cppm
@@ -0,0 +1,67 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+//
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 -x c++-header
%t/foo.h \
+// RUN: -emit-pch -o %t/foo.pch
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 %t/use.cpp
-include-pch \
+// RUN: %t/foo.pch -emit-llvm -o - | FileCheck %t/use.cpp
+
+//--- foo.h
+#ifndef FOO_H
+#define FOO_H
+typedef float __m128 __attribute__((__vector_size__(16), __aligned__(16)));
+
+static __inline__ __m128 __attribute__((__always_inline__,
__min_vector_width__(128)))
+_mm_setr_ps(float __z, float __y, float __x, float __w)
+{
+ return __extension__ (__m128){ __z, __y, __x, __w };
+}
+
+typedef __m128 VR;
+
+inline VR MakeVR( float X, float Y, float Z, float W )
+{
+ return _mm_setr_ps( X, Y, Z, W );
+}
+
+extern "C" float sqrtf(float);
+
+namespace VectorSinConstantsSSE
+{
+ float a = (16 * sqrtf(0.225f));
+ VR A = MakeVR(a, a, a, a);
+ static const float b = (16 * sqrtf(0.225f));
+ static const VR B = MakeVR(b, b, b, b);
+}
+
+#endif // FOO_H
+
+//--- use.cpp
+#include "foo.h"
+float use() {
+return VectorSinConstantsSSE::A[0] + VectorSinConstantsSSE::A[1] +
+ VectorSinConstantsSSE::A[2] + VectorSinConstantsSSE::A[3] +
+ VectorSinConstantsSSE::B[0] + VectorSinConstantsSSE::B[1] +
+ VectorSinConstantsSSE::B[2] + VectorSinConstantsSSE::B[3];
+}
+
+// CHECK: define{{.*}}@__cxx_global_var_init(
+// CHECK: store{{.*}}[[a_RESULT:%[a-zA-Z0-9]+]], ptr
@_ZN21VectorSinConstantsSSE1aE
+
+// CHECK: define{{.*}}@__cxx_global_var_init.1(
+// CHECK: [[A_CALL:%[a-zA-Z0-9]+]] = call{{.*}}@_Z6MakeVR(
+// CHECK: store{{.*}}[[A_CALL]], ptr @_ZN21VectorSinConstantsSSE1AE
+
+// CHECK: define{{.*}}@__cxx_global_var_init.2(
+// CHECK: [[B_CALL:%[a-zA-Z0-9]+]] = call{{.*}}@_Z6MakeVR(
+// CHECK: store{{.*}}[[B_CALL]], ptr @_ZN21VectorSinConstantsSSEL1BE
+
[clang] [Serialization] Read the initializer for interesting static variables before consuming it (PR #92353)
ChuanqiXu9 wrote:
> I can reproduce the failure. The problem is that the CHECK line
>
> ```
> // CHECK: [[A_CALL:%[a-zA-Z0-9]+]] = call{{.*}}@_Z6MakeVR(
> ```
>
> assumes that a value is returned. On SystemZ, the return value is passed as
> `sret` argument, and the function itself returns `void`, so the pattern does
> not match.
Oh, thanks! I never heard that before. I guess I don't need to check the call
since what I need to check is the order the variable get initialized.
I'll try to land this after the build bot gets green.
https://github.com/llvm/llvm-project/pull/92353
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Serialization] Read the initializer for interesting static variables before consuming it (PR #92353)
redstar wrote: The lines you are trying to match are: ``` call void @_Z6MakeVR(ptr dead_on_unwind writable sret(<4 x float>) align 16 %tmp, float noundef %0, float noundef %1, float noundef %2, float noundef %3) %4 = load <4 x float>, ptr %tmp, align 16 store <4 x float> %4, ptr @_ZN21VectorSinConstantsSSE1AE, align 16 ``` I am not sure what is the best way to integrate that into the patterns. https://github.com/llvm/llvm-project/pull/92353 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Serialization] Read the initializer for interesting static variables before consuming it (PR #92353)
redstar wrote:
I can reproduce the failure. The problem is that the CHECK line
```
// CHECK: [[A_CALL:%[a-zA-Z0-9]+]] = call{{.*}}@_Z6MakeVR(
```
assumes that a value is returned. On SystemZ, the return value is passed as
`sret` argument, and the function itself returns `void`, so the pattern does
not match.
https://github.com/llvm/llvm-project/pull/92353
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Serialization] Read the initializer for interesting static variables before consuming it (PR #92353)
uweigand wrote: I'm not seeing any failures with this patch on s390x with the regular check-all and check-openmp tests. Do you have a link to the failures you were seeing in the past (was that on the build bot?)? https://github.com/llvm/llvm-project/pull/92353 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Serialization] Read the initializer for interesting static variables before consuming it (PR #92353)
ChuanqiXu9 wrote: @JonasToth @redstar @uweigand @Everybody0523 Hi, this patch previously failed on clang-s390x-linux. I guess it is a pattern mismatch failure but I can't reproduce it. I am not sure if I found the wrong person. I find you from the SystemZ's group. I want to ask if you can verify the patch can pass on that platform? And if not, could you provide the generated pattern so that I can try to match it? https://github.com/llvm/llvm-project/pull/92353 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Serialization] Read the initializer for interesting static variables before consuming it (PR #92353)
llvmbot wrote:
@llvm/pr-subscribers-clang-modules
Author: Chuanqi Xu (ChuanqiXu9)
Changes
Close https://github.com/llvm/llvm-project/issues/91418
Since we load the variable's initializers lazily, it'd be problematic if the
initializers dependent on each other. So here we try to load the initializers
of static variables to make sure they are passed to code generator by order. If
we read any thing interesting, we would consume that before emitting the
current declaration.
---
Patch is 28.54 KiB, truncated to 20.00 KiB below, full version:
https://github.com/llvm/llvm-project/pull/92353.diff
3 Files Affected:
- (modified) clang/lib/Serialization/ASTReaderDecl.cpp (+26-3)
- (added) clang/test/Modules/pr91418.cppm (+67)
- (modified) clang/test/OpenMP/nvptx_lambda_capturing.cpp (+123-123)
``diff
diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp
b/clang/lib/Serialization/ASTReaderDecl.cpp
index 0c647086e304a..a6254b70560c3 100644
--- a/clang/lib/Serialization/ASTReaderDecl.cpp
+++ b/clang/lib/Serialization/ASTReaderDecl.cpp
@@ -4186,12 +4186,35 @@ void ASTReader::PassInterestingDeclsToConsumer() {
GetDecl(ID);
EagerlyDeserializedDecls.clear();
- while (!PotentiallyInterestingDecls.empty()) {
-Decl *D = PotentiallyInterestingDecls.front();
-PotentiallyInterestingDecls.pop_front();
+ auto ConsumingPotentialInterestingDecls = [this]() {
+while (!PotentiallyInterestingDecls.empty()) {
+ Decl *D = PotentiallyInterestingDecls.front();
+ PotentiallyInterestingDecls.pop_front();
+ if (isConsumerInterestedIn(D))
+PassInterestingDeclToConsumer(D);
+}
+ };
+ std::deque MaybeInterestingDecls =
+ std::move(PotentiallyInterestingDecls);
+ assert(PotentiallyInterestingDecls.empty());
+ while (!MaybeInterestingDecls.empty()) {
+Decl *D = MaybeInterestingDecls.front();
+MaybeInterestingDecls.pop_front();
+// Since we load the variable's initializers lazily, it'd be problematic
+// if the initializers dependent on each other. So here we try to load the
+// initializers of static variables to make sure they are passed to code
+// generator by order. If we read anything interesting, we would consume
+// that before emitting the current declaration.
+if (auto *VD = dyn_cast(D);
+VD && VD->isFileVarDecl() && !VD->isExternallyVisible())
+ VD->getInit();
+ConsumingPotentialInterestingDecls();
if (isConsumerInterestedIn(D))
PassInterestingDeclToConsumer(D);
}
+
+ // If we add any new potential interesting decl in the last call, consume it.
+ ConsumingPotentialInterestingDecls();
}
void ASTReader::loadDeclUpdateRecords(PendingUpdateRecord ) {
diff --git a/clang/test/Modules/pr91418.cppm b/clang/test/Modules/pr91418.cppm
new file mode 100644
index 0..33fec992439d6
--- /dev/null
+++ b/clang/test/Modules/pr91418.cppm
@@ -0,0 +1,67 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+//
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 -x c++-header
%t/foo.h \
+// RUN: -emit-pch -o %t/foo.pch
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 %t/use.cpp
-include-pch \
+// RUN: %t/foo.pch -emit-llvm -o - | FileCheck %t/use.cpp
+
+//--- foo.h
+#ifndef FOO_H
+#define FOO_H
+typedef float __m128 __attribute__((__vector_size__(16), __aligned__(16)));
+
+static __inline__ __m128 __attribute__((__always_inline__,
__min_vector_width__(128)))
+_mm_setr_ps(float __z, float __y, float __x, float __w)
+{
+ return __extension__ (__m128){ __z, __y, __x, __w };
+}
+
+typedef __m128 VR;
+
+inline VR MakeVR( float X, float Y, float Z, float W )
+{
+ return _mm_setr_ps( X, Y, Z, W );
+}
+
+extern "C" float sqrtf(float);
+
+namespace VectorSinConstantsSSE
+{
+ float a = (16 * sqrtf(0.225f));
+ VR A = MakeVR(a, a, a, a);
+ static const float b = (16 * sqrtf(0.225f));
+ static const VR B = MakeVR(b, b, b, b);
+}
+
+#endif // FOO_H
+
+//--- use.cpp
+#include "foo.h"
+float use() {
+return VectorSinConstantsSSE::A[0] + VectorSinConstantsSSE::A[1] +
+ VectorSinConstantsSSE::A[2] + VectorSinConstantsSSE::A[3] +
+ VectorSinConstantsSSE::B[0] + VectorSinConstantsSSE::B[1] +
+ VectorSinConstantsSSE::B[2] + VectorSinConstantsSSE::B[3];
+}
+
+// CHECK: define{{.*}}@__cxx_global_var_init(
+// CHECK: store{{.*}}[[a_RESULT:%[a-zA-Z0-9]+]], ptr
@_ZN21VectorSinConstantsSSE1aE
+
+// CHECK: define{{.*}}@__cxx_global_var_init.1(
+// CHECK: [[A_CALL:%[a-zA-Z0-9]+]] = call{{.*}}@_Z6MakeVR(
+// CHECK: store{{.*}}[[A_CALL]], ptr @_ZN21VectorSinConstantsSSE1AE
+
+// CHECK: define{{.*}}@__cxx_global_var_init.2(
+// CHECK: [[B_CALL:%[a-zA-Z0-9]+]] = call{{.*}}@_Z6MakeVR(
+// CHECK: store{{.*}}[[B_CALL]], ptr @_ZN21VectorSinConstantsSSEL1BE
+
+// CHECK: define{{.*}}@__cxx_global_var_init.3(
+// CHECK: store{{.*}}[[b_RESULT:%[a-zA-Z0-9]+]], ptr
@_ZN21VectorSinConstantsSSEL1bE
+
+// CHECK: @_GLOBAL__sub_I_use.cpp
+//
[clang] [Serialization] Read the initializer for interesting static variables before consuming it (PR #92353)
https://github.com/ChuanqiXu9 created
https://github.com/llvm/llvm-project/pull/92353
Close https://github.com/llvm/llvm-project/issues/91418
Since we load the variable's initializers lazily, it'd be problematic if the
initializers dependent on each other. So here we try to load the initializers
of static variables to make sure they are passed to code generator by order. If
we read any thing interesting, we would consume that before emitting the
current declaration.
>From 699da64855f147708f153c30177a1d02a4e014f7 Mon Sep 17 00:00:00 2001
From: Chuanqi Xu
Date: Wed, 15 May 2024 12:37:16 +0800
Subject: [PATCH] [Serialization] Read the initializer for interesting static
variables before consuming it
Close https://github.com/llvm/llvm-project/issues/91418
Since we load the variable's initializers lazily, it'd be
problematic if the initializers dependent on each other. So here we try to load
the initializers of static variables to make sure they are passed to
code generator by order. If we read any thing interesting, we would
consume that before emitting the current declaration.
---
clang/lib/Serialization/ASTReaderDecl.cpp| 29 ++-
clang/test/Modules/pr91418.cppm | 67 +
clang/test/OpenMP/nvptx_lambda_capturing.cpp | 246 +--
3 files changed, 216 insertions(+), 126 deletions(-)
create mode 100644 clang/test/Modules/pr91418.cppm
diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp
b/clang/lib/Serialization/ASTReaderDecl.cpp
index 0c647086e304a..a6254b70560c3 100644
--- a/clang/lib/Serialization/ASTReaderDecl.cpp
+++ b/clang/lib/Serialization/ASTReaderDecl.cpp
@@ -4186,12 +4186,35 @@ void ASTReader::PassInterestingDeclsToConsumer() {
GetDecl(ID);
EagerlyDeserializedDecls.clear();
- while (!PotentiallyInterestingDecls.empty()) {
-Decl *D = PotentiallyInterestingDecls.front();
-PotentiallyInterestingDecls.pop_front();
+ auto ConsumingPotentialInterestingDecls = [this]() {
+while (!PotentiallyInterestingDecls.empty()) {
+ Decl *D = PotentiallyInterestingDecls.front();
+ PotentiallyInterestingDecls.pop_front();
+ if (isConsumerInterestedIn(D))
+PassInterestingDeclToConsumer(D);
+}
+ };
+ std::deque MaybeInterestingDecls =
+ std::move(PotentiallyInterestingDecls);
+ assert(PotentiallyInterestingDecls.empty());
+ while (!MaybeInterestingDecls.empty()) {
+Decl *D = MaybeInterestingDecls.front();
+MaybeInterestingDecls.pop_front();
+// Since we load the variable's initializers lazily, it'd be problematic
+// if the initializers dependent on each other. So here we try to load the
+// initializers of static variables to make sure they are passed to code
+// generator by order. If we read anything interesting, we would consume
+// that before emitting the current declaration.
+if (auto *VD = dyn_cast(D);
+VD && VD->isFileVarDecl() && !VD->isExternallyVisible())
+ VD->getInit();
+ConsumingPotentialInterestingDecls();
if (isConsumerInterestedIn(D))
PassInterestingDeclToConsumer(D);
}
+
+ // If we add any new potential interesting decl in the last call, consume it.
+ ConsumingPotentialInterestingDecls();
}
void ASTReader::loadDeclUpdateRecords(PendingUpdateRecord ) {
diff --git a/clang/test/Modules/pr91418.cppm b/clang/test/Modules/pr91418.cppm
new file mode 100644
index 0..33fec992439d6
--- /dev/null
+++ b/clang/test/Modules/pr91418.cppm
@@ -0,0 +1,67 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+//
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 -x c++-header
%t/foo.h \
+// RUN: -emit-pch -o %t/foo.pch
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 %t/use.cpp
-include-pch \
+// RUN: %t/foo.pch -emit-llvm -o - | FileCheck %t/use.cpp
+
+//--- foo.h
+#ifndef FOO_H
+#define FOO_H
+typedef float __m128 __attribute__((__vector_size__(16), __aligned__(16)));
+
+static __inline__ __m128 __attribute__((__always_inline__,
__min_vector_width__(128)))
+_mm_setr_ps(float __z, float __y, float __x, float __w)
+{
+ return __extension__ (__m128){ __z, __y, __x, __w };
+}
+
+typedef __m128 VR;
+
+inline VR MakeVR( float X, float Y, float Z, float W )
+{
+ return _mm_setr_ps( X, Y, Z, W );
+}
+
+extern "C" float sqrtf(float);
+
+namespace VectorSinConstantsSSE
+{
+ float a = (16 * sqrtf(0.225f));
+ VR A = MakeVR(a, a, a, a);
+ static const float b = (16 * sqrtf(0.225f));
+ static const VR B = MakeVR(b, b, b, b);
+}
+
+#endif // FOO_H
+
+//--- use.cpp
+#include "foo.h"
+float use() {
+return VectorSinConstantsSSE::A[0] + VectorSinConstantsSSE::A[1] +
+ VectorSinConstantsSSE::A[2] + VectorSinConstantsSSE::A[3] +
+ VectorSinConstantsSSE::B[0] + VectorSinConstantsSSE::B[1] +
+ VectorSinConstantsSSE::B[2] + VectorSinConstantsSSE::B[3];
+}
+
+// CHECK: define{{.*}}@__cxx_global_var_init(
+// CHECK: store{{.*}}[[a_RESULT:%[a-zA-Z0-9]+]], ptr
