[PATCH] D57524: Fix ICE on attempt to add an addr space qual to a type qualified by an addr space
This revision was automatically updated to reflect the committed changes.
Closed by commit rC353160: Fix ICE on reference binding with mismatching addr
spaces. (authored by stulova, committed by ).
Herald added a project: clang.
Changed prior to commit:
https://reviews.llvm.org/D57524?vs=185024=185272#toc
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D57524/new/
https://reviews.llvm.org/D57524
Files:
lib/Sema/SemaInit.cpp
test/SemaOpenCLCXX/address-space-of-this.cl
Index: test/SemaOpenCLCXX/address-space-of-this.cl
===
--- test/SemaOpenCLCXX/address-space-of-this.cl
+++ test/SemaOpenCLCXX/address-space-of-this.cl
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 %s -triple spir-unknown-unknown -cl-std=c++ -pedantic
-verify -fsyntax-only
+// expected-no-diagnostics
+
+// Extract from PR38614
+struct C {};
+
+C f1() {
+ return C{};
+}
+
+C f2(){
+C c;
+return c;
+}
Index: lib/Sema/SemaInit.cpp
===
--- lib/Sema/SemaInit.cpp
+++ lib/Sema/SemaInit.cpp
@@ -4670,19 +4670,23 @@
// applied.
// Postpone address space conversions to after the temporary
materialization
// conversion to allow creating temporaries in the alloca address space.
-auto AS1 = T1Quals.getAddressSpace();
-auto AS2 = T2Quals.getAddressSpace();
-T1Quals.removeAddressSpace();
-T2Quals.removeAddressSpace();
-QualType cv1T4 = S.Context.getQualifiedType(cv2T2, T1Quals);
-if (T1Quals != T2Quals)
+auto T1QualsIgnoreAS = T1Quals;
+auto T2QualsIgnoreAS = T2Quals;
+if (T1Quals.getAddressSpace() != T2Quals.getAddressSpace()) {
+ T1QualsIgnoreAS.removeAddressSpace();
+ T2QualsIgnoreAS.removeAddressSpace();
+}
+QualType cv1T4 = S.Context.getQualifiedType(cv2T2, T1QualsIgnoreAS);
+if (T1QualsIgnoreAS != T2QualsIgnoreAS)
Sequence.AddQualificationConversionStep(cv1T4, ValueKind);
Sequence.AddReferenceBindingStep(cv1T4, ValueKind == VK_RValue);
ValueKind = isLValueRef ? VK_LValue : VK_XValue;
-if (AS1 != AS2) {
- T1Quals.addAddressSpace(AS1);
- QualType cv1AST4 = S.Context.getQualifiedType(cv2T2, T1Quals);
- Sequence.AddQualificationConversionStep(cv1AST4, ValueKind);
+// Add addr space conversion if required.
+if (T1Quals.getAddressSpace() != T2Quals.getAddressSpace()) {
+ auto T4Quals = cv1T4.getQualifiers();
+ T4Quals.addAddressSpace(T1Quals.getAddressSpace());
+ QualType cv1T4WithAS = S.Context.getQualifiedType(T2, T4Quals);
+ Sequence.AddQualificationConversionStep(cv1T4WithAS, ValueKind);
}
// In any case, the reference is bound to the resulting glvalue (or to
Index: test/SemaOpenCLCXX/address-space-of-this.cl
===
--- test/SemaOpenCLCXX/address-space-of-this.cl
+++ test/SemaOpenCLCXX/address-space-of-this.cl
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 %s -triple spir-unknown-unknown -cl-std=c++ -pedantic -verify -fsyntax-only
+// expected-no-diagnostics
+
+// Extract from PR38614
+struct C {};
+
+C f1() {
+ return C{};
+}
+
+C f2(){
+C c;
+return c;
+}
Index: lib/Sema/SemaInit.cpp
===
--- lib/Sema/SemaInit.cpp
+++ lib/Sema/SemaInit.cpp
@@ -4670,19 +4670,23 @@
// applied.
// Postpone address space conversions to after the temporary materialization
// conversion to allow creating temporaries in the alloca address space.
-auto AS1 = T1Quals.getAddressSpace();
-auto AS2 = T2Quals.getAddressSpace();
-T1Quals.removeAddressSpace();
-T2Quals.removeAddressSpace();
-QualType cv1T4 = S.Context.getQualifiedType(cv2T2, T1Quals);
-if (T1Quals != T2Quals)
+auto T1QualsIgnoreAS = T1Quals;
+auto T2QualsIgnoreAS = T2Quals;
+if (T1Quals.getAddressSpace() != T2Quals.getAddressSpace()) {
+ T1QualsIgnoreAS.removeAddressSpace();
+ T2QualsIgnoreAS.removeAddressSpace();
+}
+QualType cv1T4 = S.Context.getQualifiedType(cv2T2, T1QualsIgnoreAS);
+if (T1QualsIgnoreAS != T2QualsIgnoreAS)
Sequence.AddQualificationConversionStep(cv1T4, ValueKind);
Sequence.AddReferenceBindingStep(cv1T4, ValueKind == VK_RValue);
ValueKind = isLValueRef ? VK_LValue : VK_XValue;
-if (AS1 != AS2) {
- T1Quals.addAddressSpace(AS1);
- QualType cv1AST4 = S.Context.getQualifiedType(cv2T2, T1Quals);
- Sequence.AddQualificationConversionStep(cv1AST4, ValueKind);
+// Add addr space conversion if required.
+if (T1Quals.getAddressSpace() != T2Quals.getAddressSpace()) {
+ auto T4Quals = cv1T4.getQualifiers();
+ T4Quals.addAddressSpace(T1Quals.getAddressSpace());
+ QualType cv1T4WithAS = S.Context.getQualifiedType(T2, T4Quals);
+ Sequence.AddQualificationConversionStep(cv1T4WithAS, ValueKind);
}
// In any case, the reference
[PATCH] D57524: Fix ICE on attempt to add an addr space qual to a type qualified by an addr space
rjmccall accepted this revision. rjmccall added a comment. This revision is now accepted and ready to land. Okay. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57524/new/ https://reviews.llvm.org/D57524 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D57524: Fix ICE on attempt to add an addr space qual to a type qualified by an addr space
Anastasia updated this revision to Diff 185024.
Anastasia added a comment.
- Renamed variables and made other changes to improve readability.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D57524/new/
https://reviews.llvm.org/D57524
Files:
lib/Sema/SemaInit.cpp
test/SemaOpenCLCXX/address-space-of-this.cl
Index: test/SemaOpenCLCXX/address-space-of-this.cl
===
--- /dev/null
+++ test/SemaOpenCLCXX/address-space-of-this.cl
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 %s -triple spir-unknown-unknown -cl-std=c++ -pedantic
-verify -fsyntax-only
+// expected-no-diagnostics
+
+// Extract from PR38614
+struct C {};
+
+C f1() {
+ return C{};
+}
+
+C f2(){
+C c;
+return c;
+}
Index: lib/Sema/SemaInit.cpp
===
--- lib/Sema/SemaInit.cpp
+++ lib/Sema/SemaInit.cpp
@@ -4670,19 +4670,23 @@
// applied.
// Postpone address space conversions to after the temporary
materialization
// conversion to allow creating temporaries in the alloca address space.
-auto AS1 = T1Quals.getAddressSpace();
-auto AS2 = T2Quals.getAddressSpace();
-T1Quals.removeAddressSpace();
-T2Quals.removeAddressSpace();
-QualType cv1T4 = S.Context.getQualifiedType(cv2T2, T1Quals);
-if (T1Quals != T2Quals)
+auto T1QualsIgnoreAS = T1Quals;
+auto T2QualsIgnoreAS = T2Quals;
+if (T1Quals.getAddressSpace() != T2Quals.getAddressSpace()) {
+ T1QualsIgnoreAS.removeAddressSpace();
+ T2QualsIgnoreAS.removeAddressSpace();
+}
+QualType cv1T4 = S.Context.getQualifiedType(cv2T2, T1QualsIgnoreAS);
+if (T1QualsIgnoreAS != T2QualsIgnoreAS)
Sequence.AddQualificationConversionStep(cv1T4, ValueKind);
Sequence.AddReferenceBindingStep(cv1T4, ValueKind == VK_RValue);
ValueKind = isLValueRef ? VK_LValue : VK_XValue;
-if (AS1 != AS2) {
- T1Quals.addAddressSpace(AS1);
- QualType cv1AST4 = S.Context.getQualifiedType(cv2T2, T1Quals);
- Sequence.AddQualificationConversionStep(cv1AST4, ValueKind);
+// Add addr space conversion if required.
+if (T1Quals.getAddressSpace() != T2Quals.getAddressSpace()) {
+ auto T4Quals = cv1T4.getQualifiers();
+ T4Quals.addAddressSpace(T1Quals.getAddressSpace());
+ QualType cv1T4WithAS = S.Context.getQualifiedType(T2, T4Quals);
+ Sequence.AddQualificationConversionStep(cv1T4WithAS, ValueKind);
}
// In any case, the reference is bound to the resulting glvalue (or to
Index: test/SemaOpenCLCXX/address-space-of-this.cl
===
--- /dev/null
+++ test/SemaOpenCLCXX/address-space-of-this.cl
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 %s -triple spir-unknown-unknown -cl-std=c++ -pedantic -verify -fsyntax-only
+// expected-no-diagnostics
+
+// Extract from PR38614
+struct C {};
+
+C f1() {
+ return C{};
+}
+
+C f2(){
+C c;
+return c;
+}
Index: lib/Sema/SemaInit.cpp
===
--- lib/Sema/SemaInit.cpp
+++ lib/Sema/SemaInit.cpp
@@ -4670,19 +4670,23 @@
// applied.
// Postpone address space conversions to after the temporary materialization
// conversion to allow creating temporaries in the alloca address space.
-auto AS1 = T1Quals.getAddressSpace();
-auto AS2 = T2Quals.getAddressSpace();
-T1Quals.removeAddressSpace();
-T2Quals.removeAddressSpace();
-QualType cv1T4 = S.Context.getQualifiedType(cv2T2, T1Quals);
-if (T1Quals != T2Quals)
+auto T1QualsIgnoreAS = T1Quals;
+auto T2QualsIgnoreAS = T2Quals;
+if (T1Quals.getAddressSpace() != T2Quals.getAddressSpace()) {
+ T1QualsIgnoreAS.removeAddressSpace();
+ T2QualsIgnoreAS.removeAddressSpace();
+}
+QualType cv1T4 = S.Context.getQualifiedType(cv2T2, T1QualsIgnoreAS);
+if (T1QualsIgnoreAS != T2QualsIgnoreAS)
Sequence.AddQualificationConversionStep(cv1T4, ValueKind);
Sequence.AddReferenceBindingStep(cv1T4, ValueKind == VK_RValue);
ValueKind = isLValueRef ? VK_LValue : VK_XValue;
-if (AS1 != AS2) {
- T1Quals.addAddressSpace(AS1);
- QualType cv1AST4 = S.Context.getQualifiedType(cv2T2, T1Quals);
- Sequence.AddQualificationConversionStep(cv1AST4, ValueKind);
+// Add addr space conversion if required.
+if (T1Quals.getAddressSpace() != T2Quals.getAddressSpace()) {
+ auto T4Quals = cv1T4.getQualifiers();
+ T4Quals.addAddressSpace(T1Quals.getAddressSpace());
+ QualType cv1T4WithAS = S.Context.getQualifiedType(T2, T4Quals);
+ Sequence.AddQualificationConversionStep(cv1T4WithAS, ValueKind);
}
// In any case, the reference is bound to the resulting glvalue (or to
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D57524: Fix ICE on attempt to add an addr space qual to a type qualified by an addr space
rjmccall added inline comments. Comment at: lib/Sema/SemaInit.cpp:4693 +T2Quals.addAddressSpace(AS2); + QualType WithAScv1T4 = S.Context.getQualifiedType(IgnoreAScv2T2, T1Quals); + Sequence.AddQualificationConversionStep(WithAScv1T4, ValueKind); Anastasia wrote: > rjmccall wrote: > > `Qualifiers::addQualifiers` should let you do this in a single step. Also, > > you seem to be modifying `T2Quals` here after the last use of it. > Ok, I will update to use Qualifies methods directly. > > As for `T2Quals` I was thinking it might make sense to restore the original > value in case some code later will be added to use it (to prevent bugs)... > but may be it's too hypothetical. :) > > Thanks! That's not unreasonable, but if you're going to do that, you should do it unconditionally, not just in a `AS1 != AS2` block — or better yet, make new locals for `T1QualsWithoutAS`. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57524/new/ https://reviews.llvm.org/D57524 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D57524: Fix ICE on attempt to add an addr space qual to a type qualified by an addr space
Anastasia marked an inline comment as done. Anastasia added inline comments. Comment at: lib/Sema/SemaInit.cpp:4693 +T2Quals.addAddressSpace(AS2); + QualType WithAScv1T4 = S.Context.getQualifiedType(IgnoreAScv2T2, T1Quals); + Sequence.AddQualificationConversionStep(WithAScv1T4, ValueKind); rjmccall wrote: > `Qualifiers::addQualifiers` should let you do this in a single step. Also, > you seem to be modifying `T2Quals` here after the last use of it. Ok, I will update to use Qualifies methods directly. As for `T2Quals` I was thinking it might make sense to restore the original value in case some code later will be added to use it (to prevent bugs)... but may be it's too hypothetical. :) Thanks! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57524/new/ https://reviews.llvm.org/D57524 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D57524: Fix ICE on attempt to add an addr space qual to a type qualified by an addr space
rjmccall added inline comments. Comment at: lib/Sema/SemaInit.cpp:4693 +T2Quals.addAddressSpace(AS2); + QualType WithAScv1T4 = S.Context.getQualifiedType(IgnoreAScv2T2, T1Quals); + Sequence.AddQualificationConversionStep(WithAScv1T4, ValueKind); `Qualifiers::addQualifiers` should let you do this in a single step. Also, you seem to be modifying `T2Quals` here after the last use of it. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57524/new/ https://reviews.llvm.org/D57524 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D57524: Fix ICE on attempt to add an addr space qual to a type qualified by an addr space
Anastasia created this revision.
Anastasia added a reviewer: rjmccall.
If `cv2T2` has an address space and we are creating a qualified type `cv1T4`
adding new addr space qualifier an ICE is triggered.
To avoid this ICE we first create a duplicate of `cv2T2` omitting its original
addr space. Then we can safely use it to create combined type to use in the
conversion.
This is the last patch that allows fixing PR38614!
https://reviews.llvm.org/D57524
Files:
lib/Sema/SemaInit.cpp
test/SemaOpenCLCXX/address-space-of-this.cl
Index: test/SemaOpenCLCXX/address-space-of-this.cl
===
--- /dev/null
+++ test/SemaOpenCLCXX/address-space-of-this.cl
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 %s -triple spir-unknown-unknown -cl-std=c++ -pedantic
-verify -fsyntax-only
+// expected-no-diagnostics
+
+// Extract from PR38614
+struct C {};
+
+C f1() {
+ return C{};
+}
+
+C f2(){
+C c;
+return c;
+}
Index: lib/Sema/SemaInit.cpp
===
--- lib/Sema/SemaInit.cpp
+++ lib/Sema/SemaInit.cpp
@@ -4672,17 +4672,26 @@
// conversion to allow creating temporaries in the alloca address space.
auto AS1 = T1Quals.getAddressSpace();
auto AS2 = T2Quals.getAddressSpace();
-T1Quals.removeAddressSpace();
-T2Quals.removeAddressSpace();
+if (AS1 != AS2) {
+ T1Quals.removeAddressSpace();
+ T2Quals.removeAddressSpace();
+}
QualType cv1T4 = S.Context.getQualifiedType(cv2T2, T1Quals);
if (T1Quals != T2Quals)
Sequence.AddQualificationConversionStep(cv1T4, ValueKind);
Sequence.AddReferenceBindingStep(cv1T4, ValueKind == VK_RValue);
ValueKind = isLValueRef ? VK_LValue : VK_XValue;
if (AS1 != AS2) {
- T1Quals.addAddressSpace(AS1);
- QualType cv1AST4 = S.Context.getQualifiedType(cv2T2, T1Quals);
- Sequence.AddQualificationConversionStep(cv1AST4, ValueKind);
+ if (AS1 != LangAS::Default)
+T1Quals.addAddressSpace(AS1);
+ // If there has been an addr space qualifier on cv2T2 we will fail
+ // to create a new type. To avoid this we re-create the type ignoring
+ // addr spaces first.
+ auto IgnoreAScv2T2 = S.Context.getQualifiedType(T2, T2Quals);
+ if (AS2 != LangAS::Default)
+T2Quals.addAddressSpace(AS2);
+ QualType WithAScv1T4 = S.Context.getQualifiedType(IgnoreAScv2T2,
T1Quals);
+ Sequence.AddQualificationConversionStep(WithAScv1T4, ValueKind);
}
// In any case, the reference is bound to the resulting glvalue (or to
Index: test/SemaOpenCLCXX/address-space-of-this.cl
===
--- /dev/null
+++ test/SemaOpenCLCXX/address-space-of-this.cl
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 %s -triple spir-unknown-unknown -cl-std=c++ -pedantic -verify -fsyntax-only
+// expected-no-diagnostics
+
+// Extract from PR38614
+struct C {};
+
+C f1() {
+ return C{};
+}
+
+C f2(){
+C c;
+return c;
+}
Index: lib/Sema/SemaInit.cpp
===
--- lib/Sema/SemaInit.cpp
+++ lib/Sema/SemaInit.cpp
@@ -4672,17 +4672,26 @@
// conversion to allow creating temporaries in the alloca address space.
auto AS1 = T1Quals.getAddressSpace();
auto AS2 = T2Quals.getAddressSpace();
-T1Quals.removeAddressSpace();
-T2Quals.removeAddressSpace();
+if (AS1 != AS2) {
+ T1Quals.removeAddressSpace();
+ T2Quals.removeAddressSpace();
+}
QualType cv1T4 = S.Context.getQualifiedType(cv2T2, T1Quals);
if (T1Quals != T2Quals)
Sequence.AddQualificationConversionStep(cv1T4, ValueKind);
Sequence.AddReferenceBindingStep(cv1T4, ValueKind == VK_RValue);
ValueKind = isLValueRef ? VK_LValue : VK_XValue;
if (AS1 != AS2) {
- T1Quals.addAddressSpace(AS1);
- QualType cv1AST4 = S.Context.getQualifiedType(cv2T2, T1Quals);
- Sequence.AddQualificationConversionStep(cv1AST4, ValueKind);
+ if (AS1 != LangAS::Default)
+T1Quals.addAddressSpace(AS1);
+ // If there has been an addr space qualifier on cv2T2 we will fail
+ // to create a new type. To avoid this we re-create the type ignoring
+ // addr spaces first.
+ auto IgnoreAScv2T2 = S.Context.getQualifiedType(T2, T2Quals);
+ if (AS2 != LangAS::Default)
+T2Quals.addAddressSpace(AS2);
+ QualType WithAScv1T4 = S.Context.getQualifiedType(IgnoreAScv2T2, T1Quals);
+ Sequence.AddQualificationConversionStep(WithAScv1T4, ValueKind);
}
// In any case, the reference is bound to the resulting glvalue (or to
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
