OpenMPTarget: scratch memory implementation #3611

rgayatri23 · 2020-11-20T23:57:07Z

A basic implementation for scratch memory.
Modified the KOKKOS_IMPL_IF_ON_HOST macro to use "declare variant " directives.
Reverted the incremental test levels to match the file names.
Patrick Steinbrecher is a major collaborator on this PR .

dalg24

Did you mean to comment out code in the incremental tests?

dalg24 · 2020-11-21T03:51:14Z

core/src/OpenMPTarget/Kokkos_OpenMPTarget_Exec.hpp

  inline OpenMPTargetExecTeamMember(
      const int league_rank, const int league_size, const int team_size,
      const int vector_length  // const TeamPolicyInternal< OpenMPTarget,
                               // Properties ...> & team
      ,
      void* const glb_scratch, const int shmem_size_L1, const int shmem_size_L2)
-      : m_team_shared(nullptr, 0),
+      : m_team_shared(((char*)glb_scratch +
+                       league_rank * (shmem_size_L1 + shmem_size_L2 + 8)),


Should the pointer be shifted by another 8 bytes since you said you are saving it for reductions?

Ah yes it should be 16 to support all data types.

Did you mean to comment out code in the incremental tests?

I am not commenting any code in the incremental tests. I am only protecting an input size for the OpenMPTarget backend.

You are commenting a lot of code in core/unit_test/incremental/Test12a_ThreadScratch.hpp

Yes sorry about that. I did not commit that file 🙄 . Also made a small change in the same tests, where I replace the hard coded scratch level to a const variable.

dalg24 · 2020-11-21T03:53:20Z

core/src/OpenMPTarget/Kokkos_OpenMPTarget_Exec.hpp

@@ -733,13 +734,22 @@ class OpenMPTargetExecTeamMember {
  using space = execution_space::scratch_memory_space;

 public:
+  // FIXME: OPENMPTARGET - 8 bytes at the begining of the scratch space for each


Weren't we using FIXME_OPENMPTARGET as token?
Also typo begining -> beginning

I dint realize that. I will replace the token and fix the typo.

janciesko · 2020-11-30T20:00:10Z

core/unit_test/incremental/Test12b_TeamScratch.hpp

@@ -105,9 +107,17 @@ struct TeamScratch {

 TEST(TEST_CATEGORY, IncrTest_12b_TeamScratch) {
  TeamScratch<TEST_EXECSPACE> test;
+  // Fails with larger vector sizes and if the team-size is not a multiple
+  // of 32.
+#ifdef KOKKOS_ENABLE_OPENMPTARGET


Ok as a temporal workaround. Has the issue of failing with non-multiples of 32 been communicating to the CLANG folks?

I am not sure if this has been communicated to the clang compiler developers yet. I am in the process of writing a small reproducer for this issue to be submitted as a bug. Hopefully this is something that is easily fixable.

.jenkins

rgayatri23 · 2020-12-03T20:57:22Z

core/src/OpenMPTarget/Kokkos_OpenMPTarget_Exec.cpp

-      MAX_ACTIVE_THREADS * thread_local_bytes;  // Thread Private Scratch
+  const int64_t shmem_size =
+      shmem_size_L0 + shmem_size_L1;  // L0 + L1 scratch memory per team.
+  const int64_t padding = shmem_size * 10 / 100;  // Padding per team.


Over allocating the requested scratch memory by 10% per team to find an open slot sooner.

??? How is this supposed to work? Why #if?

kokkos/core/src/impl/Kokkos_SharedAlloc.hpp

Lines 51 to 69 in b3e6acd

// undefined at end of file

#if defined(KOKKOS_ENABLE_OPENMPTARGET)

#if defined(KOKKOS_COMPILER_PGI)

#define KOKKOS_IMPL_IF_ON_HOST if (!__builtin_is_device_code())

#else

// Note: OpenMPTarget enforces C++17 at configure time

#pragma omp begin declare variant match(device = {kind(host)})

static constexpr bool omp_is_initial_device2() { return true; }

#pragma omp end declare variant

#pragma omp begin declare variant match(device = {kind(nohost)})

static constexpr bool omp_is_initial_device2() { return false; }

#pragma omp end declare variant

#define KOKKOS_IMPL_IF_ON_HOST if constexpr (omp_is_initial_device2())

#endif

#else

#define KOKKOS_IMPL_IF_ON_HOST if (true)

#endif

Yes, you are right. It has to be #ifdef here.

…d correctly, fails at runtime.

…e scratch array is being used for reduction.

…atch routine.

no-undefined-internal : avoid warnings on internal linkage due to the is_omp_initial_device2 that is inside the declare no-pass-failed : avoids warnings arising from simd directives in debug mode.

…mory for the OpenMPTarget backend.

rgayatri23 · 2020-12-04T02:23:37Z

The PR is ready for review.

dalg24 · 2020-12-04T17:13:17Z

core/src/Kokkos_View.hpp

@@ -1618,7 +1618,7 @@ class View : public ViewTraits<DataType, Properties...> {
      : View(arg_prop,
             typename traits::array_layout(arg_N0, arg_N1, arg_N2, arg_N3,
                                           arg_N4, arg_N5, arg_N6, arg_N7)) {
-#ifdef KOKKOS_ACTIVE_EXECUTION_MEMORY_SPACE_HOST
+#if KOKKOS_IMPL_IF_ON_HOST


??? How is this supposed to work? Why #if?

kokkos/core/src/impl/Kokkos_SharedAlloc.hpp

Lines 51 to 69 in b3e6acd

// undefined at end of file

#if defined(KOKKOS_ENABLE_OPENMPTARGET)

#if defined(KOKKOS_COMPILER_PGI)

#define KOKKOS_IMPL_IF_ON_HOST if (!__builtin_is_device_code())

#else

// Note: OpenMPTarget enforces C++17 at configure time

#pragma omp begin declare variant match(device = {kind(host)})

static constexpr bool omp_is_initial_device2() { return true; }

#pragma omp end declare variant

#pragma omp begin declare variant match(device = {kind(nohost)})

static constexpr bool omp_is_initial_device2() { return false; }

#pragma omp end declare variant

#define KOKKOS_IMPL_IF_ON_HOST if constexpr (omp_is_initial_device2())

#endif

#else

#define KOKKOS_IMPL_IF_ON_HOST if (true)

#endif

Also

kokkos/core/src/impl/Kokkos_SharedAlloc.hpp

Line 519 in b3e6acd

#undef KOKKOS_IMPL_IF_ON_HOST

…oc.hpp. Removed the #if for KOKKOS_IMPL_IF_ON_HOST macro in Kokkos_View.hpp.

…enMPTarget backend.

…fined(KOKKOS_ACTIVE_EXECUTION_MEMORY_SPACE_HOST) in Kokkos_View.hpp.

crtrott · 2020-12-05T16:53:05Z

core/src/Kokkos_View.hpp

@@ -1618,7 +1618,17 @@ class View : public ViewTraits<DataType, Properties...> {
      : View(arg_prop,
             typename traits::array_layout(arg_N0, arg_N1, arg_N2, arg_N3,
                                           arg_N4, arg_N5, arg_N6, arg_N7)) {
-#ifdef KOKKOS_ACTIVE_EXECUTION_MEMORY_SPACE_HOST
+#ifdef KOKKOS_ENABLE_OPENMPTARGET


OK for now we need a better long term solution.

crtrott · 2020-12-05T16:54:29Z

core/src/OpenMPTarget/Kokkos_OpenMPTarget_Exec.cpp

+  // the number of threads per threadblock will not exceed 1024 threads. In
+  // future this value should be based on the chosen architecture for the
+  // OpenMPTarget backend.
+  int64_t total_size = (shmem_size + 16 + padding) * ((1024 * 80) / team_size);


we probably need to build our own list for this, how fun ...

also this needs to be 2048*80

The reason I used 1024 instead of 2048 is because it might be very rare that all the threads in a thread block will be used. Do you want me to use 2048 anyway?

crtrott

The "which scratch buffer position do I need to use" thing needs to be lock based since the number of teams can exceed what the hardware can have in flight concurrently, and thus you run out of space.

crtrott · 2020-12-05T16:59:26Z

core/src/OpenMPTarget/Kokkos_OpenMPTarget_Exec.hpp

+      void* const glb_scratch, const int shmem_size_L0, const int shmem_size_L1)
+      : m_team_shared(
+            ((char*)glb_scratch +
+             league_rank * (shmem_size_L0 + shmem_size_L1 +


This is not how this works: note league_rank can be larger than the 2048*80/team_size, you need to use a lock array to find an open spot like in CUDA. And then its just "open_index * (shmem_size_L0+shmem_size_L1+16)"

Uhm what about my comment here?

crtrott · 2020-12-05T16:59:48Z

core/src/OpenMPTarget/Kokkos_OpenMPTarget_Exec.cpp

+  // the number of threads per threadblock will not exceed 1024 threads. In
+  // future this value should be based on the chosen architecture for the
+  // OpenMPTarget backend.
+  int64_t total_size = (shmem_size + 16 + padding) * ((1024 * 80) / team_size);


also this needs to be 2048*80

crtrott · 2020-12-05T17:00:28Z

core/src/OpenMPTarget/Kokkos_OpenMPTarget_Exec.hpp

+    //    reduction.
+    m_reduce_scratch =
+        (char*)glb_scratch +
+        league_rank * (shmem_size_L0 + shmem_size_L1 +


same as above, needs lock based index and correct offset calculation

crtrott · 2020-12-05T17:02:14Z

core/src/impl/Kokkos_SharedAlloc.hpp

@@ -54,7 +54,15 @@
 #define KOKKOS_IMPL_IF_ON_HOST if (!__builtin_is_device_code())
 #else
 // Note: OpenMPTarget enforces C++17 at configure time
-#define KOKKOS_IMPL_IF_ON_HOST if constexpr (omp_is_initial_device())
+#pragma omp begin declare variant match(device = {kind(host)})
+static constexpr bool omp_is_initial_device2() { return true; }


lets call this differently, to make clear it is NOT a OpenMP runtime function. How about kokkos_omp_on_device()

…d omp_is_initial_device2 routine to kokkos_omp_on_device.

…m in a round-robin manner.

…ck accurately.

crtrott · 2020-12-16T17:06:37Z

core/src/OpenMPTarget/Kokkos_OpenMPTarget_Parallel.hpp

+          while (lock_array[shmem_block_index] == 0)
+            ;
+          lock_team =
+              atomic_compare_exchange(&lock_array[shmem_block_index], 1, 0);


why is the lock_team unusued?

crtrott · 2020-12-16T17:07:43Z

core/src/OpenMPTarget/Kokkos_OpenMPTarget_Parallel.hpp

+          typename Policy::member_type team(i, league_size, team_size,
+                                            vector_length, scratch_ptr,
+                                            shmem_size_L0, shmem_size_L1);
+          m_functor(team);


don't call the functor here, just compute the lock_id in the if block, then call the functor, then unlock in another if_block. Pass the lock_id to the team constructor, and if no scratch memory is used pass in a 0.

crtrott

Left comments. In particular the lock id need to be used.

…etExecTeamMember. Removed the lock-team while loop in TeamPolicy ParallelFor.

…mory to implementation to dynamically assign shared memory blocks.

crtrott · 2021-01-07T22:01:24Z

core/src/OpenMPTarget/Kokkos_OpenMPTarget_Parallel.hpp

+
+    // A lock array to keep track of scratch memory blocks.
+    // The first element to keep track of the number of free-blocks.
+    int* lock_array = new int[max_active_league_size + 1];


should be a member of the execution space instance

probably use openmpTargetSpace allocate for that and DeepCOpy to set it and then is_device_ptr later.

also add like 10% or so

probably use openmpTargetSpace allocate for that and DeepCOpy to set it and then is_device_ptr later.

I used another target region to initialize it currently. Do you think its better to set it on host and do a DeepCopy?

also add like 10% or so

Do you mean just allocate an additional 10% but not use it?

a) its fine to use a kernel to initialize, put that into a function on the execution space though. I.e. have something like "resize scratch array size"
b) No you do use it for the cycle around. Note this is also the number of scratch partitions you need to allocate.

crtrott · 2021-01-07T22:05:26Z

core/src/OpenMPTarget/Kokkos_OpenMPTarget_Parallel.hpp

+      } else {
+        // Block until there is atleast one free scratch block is available.
+        while (lock_array[0] == 0)
+          ;


This doesn't make much sense why not start with a count of 0? Also are we actually guaranteed that teams are launched in order?

and generally the convention is that "its locked" means the value is 1, and finding a 0 means its not locked.

crtrott · 2021-01-07T22:06:02Z

core/src/OpenMPTarget/Kokkos_OpenMPTarget_Parallel.hpp

+
+        // Iterate till the first free scratch block is encountered and try and
+        // get a lock on the block.
+        int iter = 1;


start at i%length_of_lock_array

so they don't step on each others feet all the time.

Modified the logic to identify a free block to incorporate all the above comments.

…he allocation of block index in scratch array.

crtrott · 2021-01-14T16:04:17Z

core/src/OpenMPTarget/Kokkos_OpenMPTarget_Parallel.hpp

+      // Loop as long as a shmem_block_index is not found.
+      while (shmem_block_index == 0) {
+        // Proceed if there are free blocks available.
+        if (lock_array[0] > 0) {


I don't think this is needed. This should ALWAYS be true. The point of the lock array is that it is actually a bit larger then the max number of active teams. So the atomic counter can never reach zero ...

crtrott · 2021-01-14T16:05:41Z

core/src/OpenMPTarget/Kokkos_OpenMPTarget_Parallel.hpp

-    lock_array[0] = max_active_league_size;
+    // Allocate the lock_array on the device and initialize it to 0's in a
+    // target region.
+    int* lock_array = static_cast<int*>(omp_target_alloc(


This ptr still needs to come from the OpenMPTarget instance, in particular so you don't reallocate it all the time. Only reallocate if it has to grow.

Yes, working on it. I am allocating the size of the lock_array to the maximum possible by assuming the team-size to 32. Since the OpenMPTarget backend has a restriction of needing a minimum of 32 threads per team, the array will not need to grow.

…be atleast one free block.

masterleinad · 2021-01-14T21:22:20Z

We have

4: Libomptarget error: Call to omp_target_memcpy with invalid arguments

in https://cloud.cees.ornl.gov/jenkins-ci/blue/organizations/jenkins/Kokkos/detail/Kokkos/4055/pipeline#step-155-log-1139 and also

4/42 Test  #4: KokkosCore_UnitTest_OpenMPTarget .............***Timeout 1500.16 sec

…ed a clear_lock routine to free the corresponding memory during finalize.

crtrott · 2021-01-19T23:09:48Z

core/src/OpenMPTarget/Kokkos_OpenMPTarget_Exec.cpp

+int* OpenMPTargetExec::get_lock_array(int num_teams) {
+  Kokkos::Experimental::OpenMPTargetSpace space;
+  int max_active_league_size = MAX_ACTIVE_THREADS / 32;
+  if (max_active_league_size < num_teams) {


that doesn't make sense. We never need more than max_active_league_size or?

I think you can just allocate MAX_ACTIVE_THREADS/32 here always. Just delete the first branch. That branch doesn't make sense anyway. This would only be true if you had a team_size < 32 .

crtrott

Almost there. Left a couple comments.

crtrott · 2021-01-19T23:15:49Z

core/src/OpenMPTarget/Kokkos_OpenMPTarget_Exec.cpp

+int* OpenMPTargetExec::get_lock_array(int num_teams) {
+  Kokkos::Experimental::OpenMPTargetSpace space;
+  int max_active_league_size = MAX_ACTIVE_THREADS / 32;
+  if (max_active_league_size < num_teams) {


I think you can just allocate MAX_ACTIVE_THREADS/32 here always. Just delete the first branch. That branch doesn't make sense anyway. This would only be true if you had a team_size < 32 .

crtrott · 2021-01-19T23:16:43Z

core/src/OpenMPTarget/Kokkos_OpenMPTarget_Parallel.hpp

+            shmem_block_index = iter;
+          else
+            iter = ++iter % max_active_teams;
+        }


This looks good now.

crtrott · 2021-01-19T23:17:27Z

core/src/OpenMPTarget/Kokkos_OpenMPTarget_Parallel.hpp

+      // Free the locked block and increment the number of available free
+      // blocks.
+      lock_team = atomic_compare_exchange(&lock_array[shmem_block_index], 1, 0);
+      atomic_increment(&lock_array[0]);


this line needs to go. leftover from the previous approach

crtrott · 2021-01-19T23:17:57Z

core/src/OpenMPTarget/Kokkos_OpenMPTarget_Parallel.hpp

+            iter = ++iter % max_active_teams;
+        }
+        // Decrement the number of available free blocks.
+        atomic_decrement(&lock_array[0]);


this line needs to go.

Yes deleted.

crtrott · 2021-01-19T23:18:34Z

core/src/OpenMPTarget/Kokkos_OpenMPTarget_Parallel.hpp

+      // Free the locked block and increment the number of available free
+      // blocks.
+      lock_team = atomic_compare_exchange(&lock_array[shmem_block_index], 1, 0);
+      atomic_increment(&lock_array[0]);


this line needs to go

rgayatri23 · 2021-01-20T01:54:25Z

I think you can just allocate MAX_ACTIVE_THREADS/32 here always. Just delete the first branch. That branch doesn't make sense anyway. This would only be true if you had a team_size < 32 .

Yes I left the logic in there to be able to address the issue in case in future we have team_size < 32

…ach. Updated the logic in get_lock_array routine.

rgayatri23 requested review from crtrott and dalg24 November 20, 2020 23:57

rgayatri23 added the [WIP] label Nov 20, 2020

dalg24 reviewed Nov 21, 2020

View reviewed changes

dalg24 requested a review from janciesko November 21, 2020 03:54

janciesko reviewed Nov 30, 2020

View reviewed changes

janciesko approved these changes Nov 30, 2020

View reviewed changes

rgayatri23 commented Dec 3, 2020

View reviewed changes

.jenkins Show resolved Hide resolved

rgayatri23 commented Dec 3, 2020

View reviewed changes

rgayatri added 10 commits December 3, 2020 18:01

Scratch trials that dont work on clang+nvidia

df00a24

Compiles the incremental scratch tests but the memory is not allocate…

b216342

…d correctly, fails at runtime.

Modified the correct offsets for scratch memory.

a0f48d1

Giving team_scratch routine the correct level.

61bcbf5

OpenMPTarget : Basic implementation for scratch memory.

c3d52e7

Adding 8 bytes offset to scratch memory since the first element of th…

b28bf27

…e scratch array is being used for reduction.

Parameterized the scratch level in incremental test and in thread_scr…

8dc728a

…atch routine.

Scratch memory allocation is updated to the specifics of Volta GPU.

7a8f84c

Edited jenkins file to avoid warnings that fail CI testing.

be50443

no-undefined-internal : avoid warnings on internal linkage due to the is_omp_initial_device2 that is inside the declare no-pass-failed : avoids warnings arising from simd directives in debug mode.

Updated TeamScratch incremental test to use less amount of scratch me…

b3e6acd

…mory for the OpenMPTarget backend.

rgayatri23 force-pushed the OpenMPTarget_scratch_trials branch from 27c8ae9 to b3e6acd Compare December 4, 2020 02:20

rgayatri23 removed the [WIP] label Dec 4, 2020

dalg24 reviewed Dec 4, 2020

View reviewed changes

rgayatri added 3 commits December 4, 2020 13:52

Removed the undef of KOKKOS_IMPL_IF_ON_HOST macro in Kokkos_SharedAll…

aada25c

…oc.hpp. Removed the #if for KOKKOS_IMPL_IF_ON_HOST macro in Kokkos_View.hpp.

Using KOKKOS_IMPL_IF_ON_HOST macro in Kokkos_View.hpp only for the Op…

76f1826

…enMPTarget backend.

Replaced elif KOKKOS_ACTIVE_EXECUTION_MEMORY_SPACE_HOST with #elif de…

0a2f796

…fined(KOKKOS_ACTIVE_EXECUTION_MEMORY_SPACE_HOST) in Kokkos_View.hpp.

crtrott reviewed Dec 5, 2020

View reviewed changes

crtrott requested changes Dec 5, 2020

View reviewed changes

Using 2048 threads for V100 instead of 1024 in resize_scratch. Rename…

bb0b062

…d omp_is_initial_device2 routine to kokkos_omp_on_device.

rgayatri added 2 commits December 9, 2020 16:45

A scratch block is assigned to a team based on the omp_target_team_nu…

f1b3baa

…m in a round-robin manner.

Renamed kokkos_omp_on_device to kokkos_omp_on_host to reflect the che…

392eb11

…ck accurately.

rgayatri23 mentioned this pull request Dec 14, 2020

omp_is_initial_device is non-constexpr at least on icpx #3661

Closed

crtrott reviewed Dec 16, 2020

View reviewed changes

crtrott requested changes Dec 16, 2020

View reviewed changes

rgayatri added 2 commits December 20, 2020 21:24

Fixed the scratch memory block index in the constructor of OpenMPTarg…

f00e4ec

…etExecTeamMember. Removed the lock-team while loop in TeamPolicy ParallelFor.

Added a base function for kokkos_omp_on_host. Modified the scratch me…

e050c55

…mory to implementation to dynamically assign shared memory blocks.

crtrott requested changes Jan 13, 2021

View reviewed changes

masterleinad linked an issue Jan 13, 2021 that may be closed by this pull request

omp_is_initial_device is non-constexpr at least on icpx #3661

Closed

Updated the lock_array allocation to use omp_target_alloc. Modified t…

a0a2aa5

…he allocation of block index in scratch array.

crtrott requested changes Jan 14, 2021

View reviewed changes

rgayatri added 2 commits January 14, 2021 09:22

Removed the tracking of empty scratch blocks since there will always …

b2f6a22

…be atleast one free block.

Added the scratch block index logic to the ParallelFor routine with tag.

3a5a393

rgayatri added 2 commits January 15, 2021 11:09

The lock_array is now created once per OpenMPTargetExec instance. Add…

0796efc

…ed a clear_lock routine to free the corresponding memory during finalize.

Adding the block index variable to team variable in ParallelReduce.

f811079

crtrott reviewed Jan 19, 2021

View reviewed changes

crtrott requested changes Jan 19, 2021

View reviewed changes

Deleted the atomic_increment and atomic_decrement from previous appro…

9953e22

…ach. Updated the logic in get_lock_array routine.

crtrott approved these changes Jan 22, 2021

View reviewed changes

dalg24 merged commit f8fd210 into kokkos:develop Jan 22, 2021

rgayatri23 deleted the OpenMPTarget_scratch_trials branch March 24, 2023 22:03

	// undefined at end of file
	#if defined(KOKKOS_ENABLE_OPENMPTARGET)
	#if defined(KOKKOS_COMPILER_PGI)
	#define KOKKOS_IMPL_IF_ON_HOST if (!__builtin_is_device_code())
	#else
	// Note: OpenMPTarget enforces C++17 at configure time
	#pragma omp begin declare variant match(device = {kind(host)})
	static constexpr bool omp_is_initial_device2() { return true; }
	#pragma omp end declare variant

	#pragma omp begin declare variant match(device = {kind(nohost)})
	static constexpr bool omp_is_initial_device2() { return false; }
	#pragma omp end declare variant

	#define KOKKOS_IMPL_IF_ON_HOST if constexpr (omp_is_initial_device2())
	#endif
	#else
	#define KOKKOS_IMPL_IF_ON_HOST if (true)
	#endif

OpenMPTarget: scratch memory implementation #3611

OpenMPTarget: scratch memory implementation #3611

Conversation

rgayatri23 commented Nov 20, 2020

dalg24 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rgayatri23 Nov 21, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rgayatri23 commented Dec 4, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

crtrott left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

crtrott left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

masterleinad commented Jan 14, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

crtrott left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rgayatri23 commented Jan 20, 2021

rgayatri23 Nov 21, 2020 •

edited

rgayatri23 commented Dec 4, 2020 •

edited