[SYCL]Emit an error if an array size exceeds SPIR-V intermediate format limitations #2093

srividya-sundaram · 2020-07-10T21:46:06Z

Some limitations in SPIR-V specification affect how SYCL code can be written.
This PR addresses one such case where we cannot access a global variable which is a const array with more than ~65532 elements from within a SYCL kernel, because of the 65,535 wordcount limit on SPIR-V instruction. Refer : Instruction-wordcount-limit
The SPIRV-LLVM-Translator was updated to prevent crashing and instead emit a helpful message about this SPIR-V limitation. PR here

According to SPIR-V specification Section 2.17.Universal-Limits :

2.17. Universal Limits
These quantities are minimum limits for all implementations and validators. Implementations are allowed to support larger quantities. Specific APIs may impose larger minimums.

While this means that a SPIR-V module can have quantities exceeding the minimum limits (65,535 WC), the device backend compiler does not handle such SPIR-V modules.

Hence, we detect quantities exceeding the limitations in device code (which is going to be translated to SPIR-V) and emit a warning.
This is not a bug, just a UX improvement in FE.

The actual text of the warning message mentioned in this PR is open for wordsmithing.

elizabethandrews

How did you generate this file - big_constant.h? Is the actual initialized array required? Will the warning be generated if you just create an array with this size and capture it? E.g. const double big[65536];

clang/lib/Sema/SemaExpr.cpp

clang/include/clang/Basic/DiagnosticSemaKinds.td

clang/lib/Sema/SemaExpr.cpp

clang/test/SemaSYCL/array-num-elements-exceeded.cpp

clang/include/clang/Basic/DiagnosticSemaKinds.td

srividya-sundaram · 2020-07-13T18:53:36Z

@elizabethandrews

Will the warning be generated if you just create an array with this size and capture it? E.g. const double big[65536];

A variable declaration which uses the const specifier should have an initializer. Otherwise, an error will be thrown along these lines: default initialization of an object of const type 'const double [65536]'

elizabethandrews · 2020-07-13T23:16:29Z

@elizabethandrews

Will the warning be generated if you just create an array with this size and capture it? E.g. const double big[65536];

A variable declaration which uses the const specifier should have an initializer. Otherwise, an error will be thrown along these lines: default initialization of an object of const type 'const double [65536]'

Oh right. My bad. I meant double [65536];

srividya-sundaram · 2020-07-14T15:31:18Z

@elizabethandrews

Will the warning be generated if you just create an array with this size and capture it? E.g. const double big[65536];

A variable declaration which uses the const specifier should have an initializer. Otherwise, an error will be thrown along these lines: default initialization of an object of const type 'const double [65536]'

Oh right. My bad. I meant double [65536];

SYCL kernel cannot use a non-const global variable. So if you are accessing a global variable from inside a Kernel, it should have const or constexpr qualifier.

From Spec:

Variables with static storage duration that are odr-used inside a kernel must be const or constexpr and
zero-initialized or constant-initialized

elizabethandrews · 2020-07-14T18:32:16Z

@elizabethandrews

Will the warning be generated if you just create an array with this size and capture it? E.g. const double big[65536];

A variable declaration which uses the const specifier should have an initializer. Otherwise, an error will be thrown along these lines: default initialization of an object of const type 'const double [65536]'

Oh right. My bad. I meant double [65536];

SYCL kernel cannot use a non-const global variable. So if you are accessing a global variable from inside a Kernel, it should have const or constexpr qualifier.

From Spec:
Variables with static storage duration that are odr-used inside a kernel must be const or constexpr and
zero-initialized or constant-initialized

I understand but why does this array size limitation apply to only global variables/constant variables, etc? If the limitation is SPIR-V instruction size, shouldn't any array captured by SYCL kernel be subject to same restrictions?

clang/test/SemaSYCL/array-num-elements-exceeded.cpp

keryell · 2020-07-14T20:29:45Z

These SPIR-V limits are super curious but seems to be still in SPIR-V 1.5 https://www.khronos.org/registry/spir-v/specs/unified1/SPIRV.html#_a_id_limits_a_universal_limits :-(
@johnkslang I guess this will start being an issue with people writing more and more complex programs in SYCL... Perhaps we should have a SPIR-V extension to push these limits up, for example to match LLVM IR?
Anyway, in the meantime our back-end use LLVM IR, not SPIR-V, and the PTX back-end does not use SPIR-V either. It is not clear in this PR whether this check is done only for the SPIR-V device target.

srividya-sundaram · 2020-07-14T20:34:37Z

These SPIR-V limits are super curious but seems to be still in SPIR-V 1.5 https://www.khronos.org/registry/spir-v/specs/unified1/SPIRV.html#_a_id_limits_a_universal_limits :-(
@johnkslang I guess this will start being an issue with people writing more and more complex programs in SYCL... Perhaps we should have a SPIR-V extension to push these limits up, for example to match LLVM IR?
Anyway, in the meantime our back-end use LLVM IR, not SPIR-V, and the PTX back-end does not use SPIR-V either. It is not clear in this PR whether this check is done only for the SPIR-V device target.

I have added the check for array size if code is compiled for SYCL devices : if(getLangOpts().SYCLIsDevice)
Can you clarify what you mean by : whether this check is done only for the SPIR-V device target.

AlexeySotkin · 2020-07-15T12:58:58Z

One more nuance. Big const arrays with zero initializer can be represented in SPIR-V as OpConstantNull. For example https://godbolt.org/z/7vhP1e :
In C/C++

int big_array[67000];
int big_array2[67000] = {};

in LLVM IR:

@big_array = dso_local global [67000 x i32] zeroinitializer, align 16
@big_array2 = dso_local global [67000 x i32] zeroinitializer, align 16

in SPIR-V:

%ulong_67000 = OpConstant %ulong 67000
%_arr_uint_ulong_67000 = OpTypeArray %uint %ulong_67000
%_ptr_Function__arr_uint_ulong_67000 = OpTypePointer Function %_arr_uint_ulong_67000
%6 = OpConstantNull %_arr_uint_ulong_67000
%big_array = OpVariable %_ptr_Function__arr_uint_ulong_67000 Function %6
%big_array2 = OpVariable %_ptr_Function__arr_uint_ulong_67000 Function %6

For this case we should not emit the error.

keryell · 2020-07-15T21:00:51Z

For this case we should not emit the error.

So perhaps the good way is to postpone the check into the LLVM-SPIRV back-end part and have some mechanism to still emit a sensible error message related to the user source code?

srividya-sundaram · 2020-07-15T23:54:25Z

@keryell @premanandrao

So this error message should be emitted only for SPIR-V device code.

I have tried to address this with my latest changes.

@AlexeySotkin @AlexeySachkov

int big_array[67000];
int big_array2[67000] = {};
For this case we should not emit the error.

Can you confirm if this should also be supported? If yes, I need to make additional changes, as this case is not currently supported.

AlexeySotkin · 2020-07-16T10:07:37Z

So perhaps the good way is to postpone the check into the LLVM-SPIRV back-end part and have some mechanism to still emit a sensible error message related to the user source code?

I don't think there is such a mechanism which allows to map LLVM IR instructions back to the user source code. The closest thing I can think of is debug info, which is not always generated/available.

AlexeySotkin · 2020-07-16T10:08:36Z

Can you confirm if this should also be supported? If yes, I need to make additional changes, as this case is not currently supported.

I confirm: it should be supported.

johnkslang · 2020-07-23T14:58:01Z

The SPIR-V limits were pushed by some who wanted to put bounds on their own implementations.

But, they are 6 years old now. I think they should be revisited and bumped up.

clang/lib/Sema/SemaSYCL.cpp

clang/test/SemaSYCL/array-num-elements-exceeded.cpp

* Due to SPIR-V intermediate format limitations, const global arrays with number of elements more than 65532 cannot be used inside SYCL kernel. * A possible workaround is to split the array into several ones.

srividya-sundaram · 2020-07-24T00:22:11Z

@bader @romanovvlad buildbot/sycl-win-x64-pr check fails for the SYCL test host-interop-task/interop-task.cpp
This test also fails in the sycl branch and is not related to my changes in this PR. I have re-run the job a couple of times and that din't help either. Whom should I report this error to?

romanovvlad · 2020-07-24T09:44:33Z

@bader @romanovvlad buildbot/sycl-win-x64-pr check fails for the SYCL test host-interop-task/interop-task.cpp
This test also fails in the sycl branch and is not related to my changes in this PR. I have re-run the job a couple of times and that din't help either. Whom should I report this error to?

Disabling the test https://github.com/intel/llvm/pull/2173/files

clang/lib/Sema/SemaSYCL.cpp

premanandrao · 2020-07-24T16:10:06Z

@elizabethandrews , can you take a look too?

elizabethandrews · 2020-07-24T16:30:15Z

clang/lib/Sema/SemaExpr.cpp

+        QualType Ty = VD->getType();
+        if (const auto *CAType = dyn_cast<ConstantArrayType>(Ty)) {
+          assert(CAType);
+          const int MaxNumElements = 65535 - 3;


I think this line can be removed and you can just check if arrayElements > 65532 below.

It should likely be constexpr at least if you're keeping the variable.

elizabethandrews · 2020-07-24T16:37:13Z

clang/lib/Sema/SemaSYCL.cpp

+bool Sema::isArrayZeroInitialized(VarDecl *VD, bool CheckValueDependent) {
+  assert(getLangOpts().SYCLIsDevice &&
+         "Should only be called during device compilation");
+  if (VD->isInvalidDecl() || !VD->hasInit() || !VD->hasGlobalStorage())


Do you need !VD->hasGlobalStorage() in here. You already checked it.

elizabethandrews · 2020-07-24T17:39:42Z

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -10908,6 +10908,9 @@ def err_sycl_restrict : Error<
  "|use a const static or global variable that is neither zero-initialized "
  "nor constant-initialized"
  "}0">;
+def err_array_num_elements_exceeded
+    : Error<"Due to SPIR-V intermediate format limitations, constant arrays with number of elements more than 65532 cannot be used in SYCL."
+            "You can workaround this limitation by splitting the array into several ones">;


I am not too fond of this diagnostic. How about -

SPIR-V intermediate format limitation (or can we just say spec here?) disallows arrays with number of elements greater than 65532. You can work around this limitation by splitting the array.

elizabethandrews · 2020-07-24T17:47:42Z

clang/lib/Sema/SemaSYCL.cpp

+  if (VD->getInit() && !VD->getInit()->isValueDependent())
+    Value = VD->evaluateValue();
+  // zero-initialized arrays do not have any initialized elements.
+  if (Value && Value->isArray() && Value->hasArrayFiller() &&


Are these additional checks required to handle the case int array2[66000] = {}; ? @erichkeane would you know if there is a better way to check this?

I think this is overly constrained, yes. If all you care about is whether the array is default initialized (as this seems to be doing?), I believe you can do this without the evaluation. You should be able to just do VD->getAnyInitializer(), which should give you an InitListExpr since this is an array type.

Then, you should be able to loop through its 'inits' to validate whether they evaluate to zero. This would also be able to catch the cases where you get int array[6] = {0,0};, which is ALSO zero initialized, but wouldn't be caught here as far as I can tell.

All this starts to become quite complex...
And what if the the array is not used at all and eliminated by some aggressive LLVM passes or if the code is optimized differently or...?
Here we make a lot of guesses ahead on what might happen quite later downstream in code-gen for one specific back-end...

Typically we don't suppress warnings based on potential optimizations. If something is illegal with NO optimizations, it also stays illegal with O3. That is, the validity of a program should never depend on the optimization settings.

I think we agree. But here we try to guess in Sema something that is unsupported by some LLVM back-ends... If I understand the title of th PR, it is not about warning but error.

I think this is overly constrained, yes. If all you care about is whether the array is default initialized (as this seems to be doing?), I believe you can do this without the evaluation. You should be able to just do VD->getAnyInitializer(), which should give you an InitListExpr since this is an array type.

Then, you should be able to loop through its 'inits' to validate whether they evaluate to zero. This would also be able to catch the cases where you get int array[6] = {0,0};, which is ALSO zero initialized, but wouldn't be caught here as far as I can tell.

Are you referring to the if checks as overly constrained here? If yes, then Value->isArray() had to be added because of assert checks in getArrayInitializedElts()

For the case int array[6] = {0,0}; , the first 2 elements will be set to 0 ( and the rest also set to 0) but, Value->getArrayInitializedElts() will be equal to 2 . This will not be a zero-initialized array and error will not be emitted for this case.
I think both the approaches would work, so how is one better than the other?

I think this is overly constrained, yes. If all you care about is whether the array is default initialized (as this seems to be doing?), I believe you can do this without the evaluation. You should be able to just do VD->getAnyInitializer(), which should give you an InitListExpr since this is an array type.
Then, you should be able to loop through its 'inits' to validate whether they evaluate to zero. This would also be able to catch the cases where you get int array[6] = {0,0};, which is ALSO zero initialized, but wouldn't be caught here as far as I can tell.

Are you referring to the if checks as overly constrained here? If yes, then Value->isArray() had to be added because of assert checks in getArrayInitializedElts()

Except you already know that the type is an array, since it is a ConstantArrayType, right?

For the case int array[6] = {0,0}; , the first 2 elements will be set to 0 ( and the rest also set to 0) but, Value->getArrayInitializedElts() will be equal to 2 . This will not be a zero-initialized array and error will not be emitted for this case.

From my reading of the code, that WILL diagnose (int array[67000] = {0,0}), since it won't be zero initialized, right? Since you'd return false in that case here. That is distinctly the incorrect behavior.

I think both the approaches would work, so how is one better than the other?
Because looping through the inits allows us to better early-escape, evaluating the entire initializer is a REALLY expensive operation, so we can do 'at most' the same amount of work by testing each initializer individually. It also gives us the opportunity to deal with defaulted constructors on record types, which have some otherwise interesting behavior.

that WILL diagnose (int array[67000] = {0,0}), since it won't be zero initialized, right

Right, but since the size(6) is < 65532, the error will not be emitted.

Note the example you quoted. I did a larger array to show that your exception here based on zero-initialized is broken. I was using the '6' simply to be different from the element count (and not 0).

If :
int array[67000] = {};
Shouldn't diagnose, neither should:
int array[67000] = {0,0,{},0};

Nor should:

DefaultConstructible DC[67000] = {};
or
DefaultConstructible DC[67000] = {{}, DefaultConstructible{}};

There is question whether
EmptyDefaultConstructible DC[67000] = {};
or
EmptyDefaultConstructible DC[67000] = {{}, EmptyDefaultConstructible{}};

Should be diagnosed.

elizabethandrews · 2020-07-24T17:57:59Z

clang/lib/Sema/SemaExpr.cpp

+               Context.getTargetInfo().getTriple().isSPIR() &&
+               !isArrayZeroInitialized(VD, /*CheckValueDependent =*/true)) {
+        QualType Ty = VD->getType();
+        if (const auto *CAType = dyn_cast<ConstantArrayType>(Ty)) {


I think you need to use getAsConstantArrayType() here instead of dyn_cast. In another PR, we ran into issues with arrays not being recognized correctly with dyn_cast when it was an elaborated type. @erichkeane can you confirm?

Yep, this could very well be a typedef wrapping the constant array type.

erichkeane · 2020-07-24T18:00:43Z

clang/lib/Sema/SemaExpr.cpp

+               Context.getTargetInfo().getTriple().isSPIR() &&
+               !isArrayZeroInitialized(VD, /*CheckValueDependent =*/true)) {
+        QualType Ty = VD->getType();
+        if (const auto *CAType = dyn_cast<ConstantArrayType>(Ty)) {


Yep, this could very well be a typedef wrapping the constant array type.

erichkeane · 2020-07-24T18:01:26Z

clang/lib/Sema/SemaExpr.cpp

+        QualType Ty = VD->getType();
+        if (const auto *CAType = dyn_cast<ConstantArrayType>(Ty)) {
+          assert(CAType);
+          const int MaxNumElements = 65535 - 3;


It should likely be constexpr at least if you're keeping the variable.

erichkeane · 2020-07-24T18:03:06Z

clang/lib/Sema/SemaSYCL.cpp

@@ -2077,6 +2077,21 @@ void Sema::finalizeSYCLDelayedAnalysis(const FunctionDecl *Caller,
  }
 }

+bool Sema::isArrayZeroInitialized(VarDecl *VD, bool CheckValueDependent) {


what is the purpose of CheckValueDependent here? It doesn't seem to be used. Additionally, you likely don't need to use it up the call chain either.

erichkeane · 2020-07-24T18:08:28Z

clang/lib/Sema/SemaSYCL.cpp

+  if (VD->getInit() && !VD->getInit()->isValueDependent())
+    Value = VD->evaluateValue();
+  // zero-initialized arrays do not have any initialized elements.
+  if (Value && Value->isArray() && Value->hasArrayFiller() &&


I think this is overly constrained, yes. If all you care about is whether the array is default initialized (as this seems to be doing?), I believe you can do this without the evaluation. You should be able to just do VD->getAnyInitializer(), which should give you an InitListExpr since this is an array type.

Then, you should be able to loop through its 'inits' to validate whether they evaluate to zero. This would also be able to catch the cases where you get int array[6] = {0,0};, which is ALSO zero initialized, but wouldn't be caught here as far as I can tell.

erichkeane · 2020-07-24T18:09:11Z

clang/test/SemaSYCL/array-num-elements-exceeded.cpp

+// RUN: %clang_cc1 -fsycl -fsycl-is-device -triple spir64-unknown -verify -fsyntax-only %s
+
+const double big[67000] = {1.0, 2.0, 3.0};
+const int zero_init_array[67000] = {};


another good repro is:

const int zero_init_array2[67000] = {0,0,0,constexpr_fun_that_returns_0()};

If all you care about is whether the array is default initialized

We care whether zeroinitializer for the array will be generated in LLVM IR. Is there a good method to detect such case on the Sema level?

Yeah, you'd have to detect the actual initializers. See: https://godbolt.org/z/Yo5bcx

You probably ALSO care about this case: https://godbolt.org/z/v8MKrM

Note that the default constructed "S2" is undef, and I'm not particularly sure what differentiates that. For example, a defaulted constructor on the empty type results in it still being 'undef', but an actual constructor will zero-init and call the constructor at global_var_init: https://godbolt.org/z/fvzK97

Fznamznon · 2020-07-27T13:25:39Z

clang/lib/Sema/SemaExpr.cpp

+      // Emit an error if a const global array's size exceeds SPIR-V
+      // intermediate format limitations for SPIRV compatible devices.


Why you diagnose only global arrays usage?

Fznamznon · 2020-07-27T13:27:43Z

clang/lib/Sema/SemaExpr.cpp

+               !isArrayZeroInitialized(VD, /*CheckValueDependent =*/true)) {
+        QualType Ty = VD->getType();
+        if (const auto *CAType = dyn_cast<ConstantArrayType>(Ty)) {
+          assert(CAType);


This assertion is weird. If CAType is null, you won't enter this branch at all.

Fznamznon · 2020-07-27T13:30:06Z

clang/lib/Sema/SemaSYCL.cpp

+  assert(getLangOpts().SYCLIsDevice &&
+         "Should only be called during device compilation");


I don't see anything SYCL-specific in this function. Should we really restrict it for using by other languages/models?

Fznamznon · 2020-07-27T13:40:43Z

clang/lib/Sema/SemaExpr.cpp

+      // Emit an error if a const global array's size exceeds SPIR-V
+      // intermediate format limitations for SPIRV compatible devices.
+      else if (IsConst && VD->hasGlobalStorage() &&
+               Context.getTargetInfo().getTriple().isSPIR() &&


Checking that the target triple is spir-based triple does not guarantee that the target output is SPIR-V. I'm not sure if we use device code output in non-SPIR-V form somewhere, but technically it will be weird if someone will try to emit valid LLVM IR for device code and get an error about something that is not supported in SPIR-V.
However I'm not sure that it is possible to understand that the end target is SPIR-V from FE perspective at all, since SPIR-V translation is happening inside a separate tool on the other compilation stage.
I think possible solution is a way to tell the FE that this code will be converted into SPIR-V, so the driver will do it when SPIR-V emission is required.

Signed-off-by: Lu, John <[email protected]> Original commit: KhronosGroup/SPIRV-LLVM-Translator@201b782

srividya-sundaram requested review from elizabethandrews, Fznamznon and premanandrao as code owners July 10, 2020 21:46

srividya-sundaram requested review from AlexeySachkov and AlexeySotkin July 10, 2020 21:48

elizabethandrews requested changes Jul 10, 2020

View reviewed changes

clang/lib/Sema/SemaExpr.cpp Outdated Show resolved Hide resolved

clang/lib/Sema/SemaExpr.cpp Show resolved Hide resolved

premanandrao reviewed Jul 13, 2020

View reviewed changes

srividya-sundaram changed the title ~~[SYCL]Emit a warning if an array size exceeds SPIR-V intermediate format limitations~~ [SYCL]Emit an error if an array size exceeds SPIR-V intermediate format limitations Jul 14, 2020

srividya-sundaram force-pushed the array-num-limit branch from 80ed991 to e9049c3 Compare July 14, 2020 19:19

premanandrao reviewed Jul 14, 2020

View reviewed changes

clang/test/SemaSYCL/array-num-elements-exceeded.cpp Outdated Show resolved Hide resolved

srividya-sundaram changed the title ~~[SYCL]Emit an error if an array size exceeds SPIR-V intermediate format limitations~~ [WIP] [SYCL]Emit an error if an array size exceeds SPIR-V intermediate format limitations Jul 15, 2020

srividya-sundaram force-pushed the array-num-limit branch from e9049c3 to 850735f Compare July 15, 2020 23:48

srividya-sundaram marked this pull request as draft July 16, 2020 04:07

premanandrao reviewed Jul 23, 2020

View reviewed changes

clang/lib/Sema/SemaSYCL.cpp Outdated Show resolved Hide resolved

clang/test/SemaSYCL/array-num-elements-exceeded.cpp Outdated Show resolved Hide resolved

[SYCL]Emit an error if array size exceeds 65532 for SPIRV targets

b671be9

* Due to SPIR-V intermediate format limitations, const global arrays with number of elements more than 65532 cannot be used inside SYCL kernel. * A possible workaround is to split the array into several ones.

srividya-sundaram force-pushed the array-num-limit branch from 463cb78 to b671be9 Compare July 23, 2020 21:27

srividya-sundaram marked this pull request as ready for review July 23, 2020 23:53

srividya-sundaram changed the title ~~[WIP] [SYCL]Emit an error if an array size exceeds SPIR-V intermediate format limitations~~ [SYCL]Emit an error if an array size exceeds SPIR-V intermediate format limitations Jul 23, 2020

premanandrao reviewed Jul 24, 2020

View reviewed changes

clang/lib/Sema/SemaSYCL.cpp Outdated Show resolved Hide resolved

Remove unwanted braces around 'if'

058241b

premanandrao approved these changes Jul 24, 2020

View reviewed changes

srividya-sundaram requested a review from elizabethandrews July 24, 2020 16:13

elizabethandrews reviewed Jul 24, 2020

View reviewed changes

erichkeane reviewed Jul 24, 2020

View reviewed changes

Fznamznon reviewed Jul 27, 2020

View reviewed changes

srividya-sundaram closed this Sep 18, 2020

jsji pushed a commit that referenced this pull request Aug 11, 2023

Fix checks with incorrect labels (#2093)

ba03f12

Signed-off-by: Lu, John <[email protected]> Original commit: KhronosGroup/SPIRV-LLVM-Translator@201b782

		// Emit an error if a const global array's size exceeds SPIR-V
		// intermediate format limitations for SPIRV compatible devices.

		assert(getLangOpts().SYCLIsDevice &&
		"Should only be called during device compilation");

[SYCL]Emit an error if an array size exceeds SPIR-V intermediate format limitations #2093

[SYCL]Emit an error if an array size exceeds SPIR-V intermediate format limitations #2093

Uh oh!

Conversation

srividya-sundaram commented Jul 10, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

elizabethandrews left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

srividya-sundaram commented Jul 13, 2020

Uh oh!

elizabethandrews commented Jul 13, 2020

Uh oh!

srividya-sundaram commented Jul 14, 2020

Uh oh!

elizabethandrews commented Jul 14, 2020

Uh oh!

Uh oh!

keryell commented Jul 14, 2020

Uh oh!

srividya-sundaram commented Jul 14, 2020

Uh oh!

AlexeySotkin commented Jul 15, 2020

Uh oh!

keryell commented Jul 15, 2020

Uh oh!

srividya-sundaram commented Jul 15, 2020

Uh oh!

AlexeySotkin commented Jul 16, 2020

Uh oh!

AlexeySotkin commented Jul 16, 2020

Uh oh!

johnkslang commented Jul 23, 2020

Uh oh!

Uh oh!

Uh oh!

srividya-sundaram commented Jul 24, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

romanovvlad commented Jul 24, 2020

Uh oh!

Uh oh!

premanandrao commented Jul 24, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

elizabethandrews Jul 24, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

elizabethandrews Jul 24, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

srividya-sundaram Jul 27, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

srividya-sundaram commented Jul 10, 2020 •

edited

Loading

srividya-sundaram commented Jul 24, 2020 •

edited

Loading

elizabethandrews Jul 24, 2020 •

edited

Loading

elizabethandrews Jul 24, 2020 •

edited

Loading

srividya-sundaram Jul 27, 2020 •

edited

Loading