Initial introduction of zfinx #75

abukharmeh · 2020-11-16T11:29:08Z

This request introduces ZFinX extension (https://github.com/riscv/riscv-zfinx/blob/master/Zfinx_spec.adoc) to the model

Huawei Technologies Research & Development (Uk) Limited, November 2020

scottj97

This is a lot to review in a single commit. It would be preferable to break it up into many smaller commits.
This appears to include changes to support XLEN != FLEN. Is that so? If so that should be a separate commit or even a separate pull request.
There is extreme copypasta in the implementation. I'm not that familiar with the Sail language but I hope there is a better way to do this than copy/pasting the same code all over. I've added individual comments on several of them but there are many more cases that I did not comment.

model/riscv_insts_fext.sail

pmundkur · 2020-11-20T00:35:50Z

Thanks for implementing the Sail model of the Zfinx (or ZfinX?) extension, and for your pull request!

Some suggestions to improve your Zfinx model:

I agree with Scott's suggestion to reduce the amount of duplication. One way to do that would be to write functions that implement your requirements for the register reads and writes, and put them in riscv_fdext_regs.sail. That file already contains a function that abstracts handling the write to the fcsr register to avoid similar duplication.
It would be nice to have a single pair of nan-boxing/unboxing functions that handles the enabled and disabled cases of Zfinx, instead of two separate custom ones. If it makes it easier, you could move the current nan-boxing/unboxing functions into riscv_fdext_regs.sail.
The Zfinx spec says the extension "removes" all floating point load/store instructions (e.g. FLW, FSW) and moves between integer and floating-point registers. However, your Sail model does not modify these instructions.

If the intent of the word "remove" in spec is these instructions are now illegal, you can modify the encdec clauses of STORE_FP/LOAD_FP to ensure that Zfinx is disabled (similarly to how they currently handle RV32/RV64). The same would need to be done for the various versions of FMV.
The Zfinx spec says that the assembly syntax of the F/D instructions would need to refer to the integer registers instead; this would require adjusting the register naming functions in riscv_fdext_regs.sail to handle Zfinx.
The discovery aspect of the spec doesn't seem to be handled (i.e. adjusting the values of mstatus.FS and misa.F).
If you have executable test cases for these instructions, it would be great if you could add them under test/zfinx-tests.
If you need to do any restructuring or code movement (e.g. moving nan-boxing to riscv_fdext_regs.sail) that is independent of Zfinx, breaking these out into independent commits is a good idea, as Scott suggests.

There are minor syntactic issues like a few cases of broken indentation and trailing whitespace (please remove!) that could be made when incorporating these suggestions.

abukharmeh · 2020-11-25T11:11:50Z

Hello Prashanth,

Thank you very much for your comments, they were quite helpful. I am currently working on all the issues listed, and I faced an Issue I would like your input on.

I am trying to implement a global immutable variable of type bit, that is assigned to the system argument that we used to enable ZFinx, and then use that for mapping statements through out the code.
e.g let zfinx_en = bool_to_bits(sys_enable_zfinx())
However, I discovered that if this is used in the global scope, the value stays uninitialised. If I use the same code to assign a local variable, it gets the correct value..

Quickly reading through the generated C emulator, it appears that global variables are assigned before emulator arguments are parsed, if this is the case, how is it possible to assign an immutable global variable to a value passed as an argument to the emulator ?

Kind regards,
Ibrahim

pmundkur · 2020-11-25T18:14:00Z

One way is to to make it a register, and initialize it in init_sys(). But it would be good to avoid creating registers if not strictly necessary.

abukharmeh · 2020-11-25T19:03:21Z

This is what I was thinking of doing, but was wondering what is the reason behind initialising the model before parsing the input arguments ?

Its not strictly necessary, however since I need that value in many mapping statements, and I would want to avoid calling the function and converting its returned value on every floating point operation, I was thinking it make sense just to have a global immutable variable always assigned the correct value in the correct representations, especially since this value cannot be changed midway through the emulation. (This would reduce the repetition in the code significantly and would increase its performance since we are not calling a C function on every instance we need that value)

pmundkur · 2020-11-30T19:48:10Z

I see what you mean. To answer your earlier question about initialization, the simulator does a consistency check to ensure that it loads an elf file that matches the RV32/RV64 architecture it was generated for. To do this, it needs to initialize the xlen_val value; so the simulator has a preinit_sail() where all such global variables are initialized before the command line arguments are parsed. You could set the value of zfinx_en from C once you know what it should be, after parsing the command line args; but that would make sys_enable_zfinx() redundant.

I'd use the less performant and repetitious way for now. The way to configure the model will probably change anyway to get these values from a yaml file.

abukharmeh · 2020-12-03T21:40:28Z

I fixed all of the issues that were pointed out earlier, but I am stuck on trying to reduce the repetitions in the code, and I am wondering if you can help me with this ?

I am trying to use forall to make a single function that would return the correct register depending on if the zfinx is enabled, the following function works correctly, however I am not sure how one would do such implementation using bits(1) or Boolean input as that cannot be size dependent as far as I am concerned, any input would be appreciated ?

val      f_reg_zfinx : forall 'n, ('n==1). (int('n),bits(5)) -> bits(flen) effect  {escape, rreg}
function f_reg_zfinx(zfinx_en,index) =    X(index) 
val      f_reg_norm :  forall 'n, ('n==0). (int('n),bits(5)) -> bits(xlen) effect  {escape, rreg}
function f_reg_norm(zfinx_en,index) =   F(index) 
overload f_reg = {f_reg_zfinx,f_reg_norm}

The following was also my other attempt at implementing a function that would return the correct register, however similar to the previous one, it needs int' !

val f_reg : forall ('n: Int), ('n==0 | 'n==1). (bits(5),int('n)) -> bits(('n*xlen) + ((1-'n)*flen)) effect {escape, rreg}
 function f_reg (Xid,n)  =
   if (n==1)  
   then  X(Xid)
   else  F(Xid)

ReYAML configuration: That would be really really nice as it would eliminate a number of issues I faced when working with the model

ReSetting Variable from C: It appears that the model assumes that the generated C variables names are consistent between builds, is there anything that guarantees such?

abukharmeh · 2020-12-21T15:23:52Z

The following issues were addressed in the previous two commits

Updates the register names printed in trace for when ZFINX is enabled. (Since instruction are handled with scattered mapping statements, it’s hard to introduce any function that would handle the register name change for ZFINX unless we have an immutable global variable that tells if ZFINX is enabled, however since the model initialise and assign all global variables before parsing the input command line arguments, it is impossible to assign such global variable with command line arguments (Initial introduction of zfinx #75 (comment)). To overcome this issue, I directly assign the flag value from C after argument parsing, this is not the best solution, however, it appears that it’s the only one if I want to avoid duplicating every single mapping statement for floating point operations.
Remove floating point loads, stores and moves.
Updates the handling of mstatus.FS and mstatus.F
Clear the code a bit (in an effort to reduce code duplications).
Adds the test binaries we used to test the introduced functionality (Please note that the objdump files have the incorrect syntax for the register naming, as they were created using an old toolchain, I will update them in a next commit along with more code refactoring when I get some help with forall).

.gitignore

Makefile

c_emulator/riscv_platform.c

c_emulator/riscv_platform.h

Makefile

model/riscv_insts_fext.sail

model/riscv_sys_control.sail

abukharmeh · 2021-02-22T17:53:53Z

Hello Jessica,

Thank you very much for your comments. Sorry for late reply, I was trying to see what will happen with this issue "riscvarchive/riscv-zfinx#8 " before replying.

Regarding formatting issues (trailing white spaces, empty lines, etc), I am sorry for these, I will go through the PR and make sure I fix them all.

Regarding other issues, please check my inline replies.

Kind regards,
Ibrahim.

abukharmeh · 2021-10-21T09:26:28Z

Hello everyone, I am back working on this, and I have managed to make all the required changes in just couple of overloaded functions in a single place. I will try to update the PR soon. Regards, Ibrahim.

Adds new mapping to swap register names, and use it in all assembly clauses

…ters for floating points Changes register accessed for floating point instructions and modify nan boxing functions for zfinx

abukharmeh · 2021-10-26T17:59:04Z

Hi everyone,
I am still working on this, thus its not final, please don't review it yet.
Kind regards,
Ibrahim.

Remove couple of misplaced whitespace, unnecessary parens

abukharmeh · 2021-10-27T10:38:37Z

Hello everyone,

@jrtc27 @pmundkur @scottj97
I tried to resolve all the issues from last time. Could you please review it, and let me know if there is anything else I should change or improve before merging this PR?

Short overview: This PR introduce an enable flag for zfinx extension, makes floating point instructions use general purpose register file , and hardwire misa.{F,D} and mstatus.FS to 0 (when zfinx is enabled).

Kind regards,
Ibrahim.

jrtc27 · 2021-10-27T11:03:44Z

Please properly format your code; you have missing spaces, too many spaces, inconsistent indentation and inconsistent use of line breaks that make it difficult to read

martinberger · 2021-10-27T11:24:22Z

Please properly format your code;

Do we have a guideline what the code-format should be?

jrtc27 · 2021-10-27T11:25:19Z

Please properly format your code;

Do we have a guideline what the code-format should be?

No, but it's easy to compare code you write against code that exists in the same file and note that it's not formatted the same way wrt spaces after commas, indentation, etc

jrtc27 · 2021-10-27T11:26:18Z

(nor even consistent between functions added in this PR next to each other in the same file)

abukharmeh · 2021-10-27T11:36:01Z

I found couple of inconsistent indentation in insts_dext file that I already fixed. I will have a good look and try to find any other inconstancies and fix them.

martinberger · 2021-11-17T14:01:55Z

No. It is not yet ready. And I do not appreciate the timeline being forced on me, that’s not appropriate. I will leave a proper review when I get the chance.

"To achieve great things, two things are needed; a plan, and not quite enough time." Attributed to many, including Leonard Bernstein.

When do you expect that Ibrahim can exhale? @jrtc27

tariqkurd-repo · 2021-11-17T15:04:48Z

@jrtc27 we can't afford to spend more time on this. Due to our internal policies it is very time consuming making changes and submitting patches, which need to also go through an internal approval flow. Unless there are actual bugs in the code then we need to merge it now. If the suggestions are stylistic then they can be fixed after the merge. We have been trying to merge this for more than 1 year now, your feedback has been invaluable and has been the only way to make the code reusable, scalable and to reduce repetition (as the SAIL documentation is very limited you are our authority on how to write it) and I thank you for this. But now we need to move on and get it merged. @abukharmeh has a lot of other work to complete this year.

jrtc27 · 2021-11-17T15:06:00Z

@jrtc27 we can't afford to spend more time on this. Due to our internal policies it is very time consuming making changes and submitting patches, which need to also go through an internal approval flow. Unless there are actual bugs in the code then we need to merge it now. If the suggestions are stylistic then they can be fixed after the merge. We have been trying to merge this for more than 1 year now, your feedback has been invaluable and has been the only way to make the code reusable, scalable and to reduce repetition (as the SAIL documentation is very limited you are our authority on how to write it) and I thank you for this. But now we need to move on and get it merged. @abukharmeh has a lot of other work to complete this year.

No. Internal policy issues are not an excuse to force others to approve sub-standard patches. That's your problem.

jrtc27 · 2021-11-17T15:07:27Z

And if you can't drive the patch to completion, then hand it off to someone else in the community to finish. I'm not merging bad code.

tariqkurd-repo · 2021-11-17T15:10:45Z

If you think the code is bad after all this effort then I have no words to describe your attitude. @martinberger what you do next is up to you. We will not submit another patch if this is what we have to put up with. Also I may remind you @jrtc27 that you don't merge the code, that's not your job.

jrtc27 · 2021-11-17T15:12:59Z

If you think the code is bad after all this effort then I have no words to describe your attitude. @martinberger what you do next is up to you. We will not submit another patch if this is what we have to put up with.

There are clear issues still with its style, the main one being the fact that it turns F(rd) = ... into an opaque function call without the = rather than using the register assignment style, so it's totally unclear that it's actually assigning to a register.

martinberger · 2021-11-17T15:42:38Z

We have over a dozen specs being ratified this week ... we need to get going ...

This reverts commit c5e62ea.

jrtc27 · 2021-11-17T15:44:05Z

We have over a dozen specs being ratified this week ... we need to get going ...

Most of which have no Sail PRs

jrtc27 · 2021-11-17T15:44:25Z

There is no rush for Zfinx over and above anything else. The style changes will be simple to fix, this can wait until that happens.

jrtc27 · 2021-11-17T15:46:10Z

I have reverted the merge, but will just take it upon myself to re-file a fixed PR since nobody else wants to

jrtc27 · 2021-11-17T15:46:34Z

(i.e. it will reappear soon, I just want to keep master clean, so don't get too upset with me...)

martinberger · 2021-11-17T15:56:19Z

Thanks for your help.

We need to avoid the impression that the Sail model is a bottleneck holding RISCV back, because otherwise some communities might not bother.

There is little Sail knowledge outside Cambridge, and not much documentation on Sail, and the expected way in which to write the RISCV model. The average computer architect has not come across liquid types before ... For this reason, it's unavoidable that the code that we get might be less than perfect. Raising a sustainable Sail/RISCV community will take a while.

martinberger · 2021-11-17T15:57:40Z

There is no rush for Zfinx over and above anything else. The style changes will be simple to fix, this can wait until that happens.

Have you got any recommendations for an order in which to merge the PRs?

jrtc27 · 2021-11-17T15:58:46Z

Thanks for your help.

We need to avoid the impression that the Sail model is a bottleneck holding RISCV back, because otherwise some communities might not bother.

There is little Sail knowledge outside Cambridge, and not much documentation on Sail, and the expected way in which to write the RISCV model. The average computer architect has not come across liquid types before ... For this reason, it's unavoidable that the code that we get might be less than perfect. Raising a sustainable Sail/RISCV community will take a while.

I realise and understand that, which is why I don't mind taking the time to help people. What I take issue with is the "I don't have time to fix my code so it should just be merged" attitude etc; had it instead be "I don't have time to fix my code can someone please take it over the line for me" it would've been a different story (i.e. what I'm now doing to attempt to keep the peace).

jrtc27 · 2021-11-17T16:00:50Z

There is no rush for Zfinx over and above anything else. The style changes will be simple to fix, this can wait until that happens.

Have you got any recommendations for an order in which to merge the PRs?

I don't think there's much of an interdependence, so just whenever things become ready. I suspect the Zb* PRs will soon be merge candidates, those were looking close to ready last I checked, though I haven't closely been following the recent changes. I've just been a bit more swamped with work and life things the past week or so compared with normal so not reviewing as promptly as I'd like.

scottj97 · 2021-11-17T16:06:16Z

I don't know who any of you are but thank you @jrtc27 for your efforts at keeping the model clean. I concur that more documentation and examples could help people write Sail code but that's no excuse for merging substandard code. Of the year this PR has been open, 8 months of that was waiting for @abukharmeh to respond to feedback (Feb to Oct), so the long time frame is no excuse for merging prematurely.

The original PR was quite nasty and would have made a mess of this repo. We must have people like @jrtc27 to uphold quality standards or this project will not survive long-term.

scottj97 · 2021-11-17T17:41:43Z

model/riscv_insts_fext.sail

+val supports_RV32D_or_RV64D : forall 'n, 'n > 0. (implicit('n), vector('n, dec, bits(5))) -> bool effect {rreg}
+function supports_RV32D_or_RV64D (reg_vec_len, registers) = {
+  if ~ (sys_enable_zfinx()) then is_RV32D_or_RV64D()
+  else if sys_enable_zfinx() & is_RV64D() then true


No need to check sys_enable_zfinx() again here; we already know it's true

Yep, hadn't even seen that code yet. The register checking should also be a different function, it's not related to whether RV32D or RV64D are supported.

scottj97 · 2021-11-17T17:43:06Z

model/riscv_insts_fext.sail

@@ -673,22 +659,22 @@ mapping clause encdec =
 /* Execution semantics ================================ */

 function clause execute (F_UN_RM_TYPE_S(rs1, rm, rd, FSQRT_S)) = {
-  let rs1_val_S = nan_unbox (F(rs1));
+  let rs1_val_S = X_or_F_S(sys_enable_zfinx(), rs1);


Can't X_or_F_S() call sys_enable_zfinx() itself? Unfortunate that we have to keep repeating this all over the place.

Yeah that's one of the things I'll be changing

(I suspect that's why the register assignment syntax was abandoned, as I could believe Sail not supporting that when you have more than one argument since that doesn't really make sense normally)

Hi, thank you for your valuable help. Its indeed why the register assignment was abandoned, I was passing Zfinx status as argument to make it obvious what's happening if this was to be placed as part of a manual of some sort.

scottj97 · 2021-11-17T17:44:38Z

model/riscv_sys_regs.sail

@@ -144,6 +144,8 @@ val sys_enable_writable_misa = {c: "sys_enable_writable_misa", ocaml: "Platform.
 val sys_enable_rvc = {c: "sys_enable_rvc", ocaml: "Platform.enable_rvc", _: "sys_enable_rvc"} : unit -> bool
 /* whether misa.{f,d} were enabled at boot */
 val sys_enable_fdext = {c: "sys_enable_fdext", ocaml: "Platform.enable_fdext", _: "sys_enable_fdext"} : unit -> bool
+/* whether zfinx was enabled at boot */
+val sys_enable_zfinx = {c: "sys_enable_zfinx",ocaml: "Platform.enable_zfinx", _: "sys_enable_zfinx"} : unit -> bool                       


Space missing after comma

scottj97 · 2021-11-17T17:46:59Z

model/riscv_fdext_regs.sail

@@ -363,6 +456,6 @@ function accrue_fflags(flags) = {
  then {
    fcsr->FFLAGS() = f;
    update_softfloat_fflags(f);
-    dirty_fd_context();
+    if ~ (sys_enable_zfinx()) then dirty_fd_context();


There are three places here that consider sys_enable_zfinx() before calling dirty_fd_context(); could that qualification be inside dirty_fd_context() instead?

I think both are ok, I could go either way on it, though I think I prefer this, since there is no FD context on Zfinx and there should probably be an assert in dirty_fd_context that it's not Zfinx (haven't checked if this patch adds it or not).

Rename the function to dirty_fd_context_if_exists() then? It's not great to keep repeating the conditional like this. Whether those bits exist or not should be defined in exactly one place.

jrtc27 · 2021-11-17T18:03:11Z

One bug I've already found going through the diff and cleaning it up is that this NaN boxes on Zfinx, i.e. always puts 32 ones in the upper half of X registers when writing a single-precision float on RV64, whereas for Zfinx it's different to F and is supposed to sign-extend. It is at least doing the right thing for the other half of that, namely ignoring the upper 32 bits rather than ensuring they are all ones like for RV32D.

Alasdair · 2021-11-17T21:16:50Z

I wonder if there are any lint-style warnings we could add to Sail itself that might help catch common style issues and pitfalls? I'd be interested in suggestions both from those writing PRs and reviewing them - I'm not involved in either so it can hard for me to have a good perspective on this. If some amount of effort from me could possible save time getting PRs into sail-riscv it might be worthwhile?

martinberger · 2021-11-18T09:34:32Z

@Alasdair There are two distinct issues that it might be worthwhile to think about separately.

Sail format (e.g. how to use brackets, indentation)
RISCVism (e.g. how do we access registers, throwing an exception vs using an option type, code-repetition, use of type-refinements)

The former would be solved by a code formatter. The latter is more substantial, but probably not so easy to write linters for.

martinberger · 2021-11-19T16:55:42Z

I had several undergraduate interns from Cambridge and Imperial this summer, who did quite a bit of Sail work. They have gone back to university, but they might be interested in doing paid Sail work. They'd be ideal for this, but they would probably want to be paid (or at least get some recognition) in some form of shape.

If there was interest in this, I'll contact them!

@Alasdair @jrtc27 @tariqkurd-repo @PeterSewell @allenjbaum @cetola @jjscheel

abukharmeh · 2021-11-19T18:05:10Z

@jrtc27 Re Nanboxing, the original spec was to always nanbox but not check for it in zfinx, it appears that it got changed to sign extend, and I didn't notice it. If I fix that along and remove the redundant check in supports_RV32_or_RV64D function. Would you be happy to merge this PR ? I am not too keen on changing X_or_F functions to remove the status flag from the arguments as that would make it less obvious what is happening !

bilalsakhawat · 2021-11-19T18:06:56Z

Hi!
I started learning sail roughly two months ago. I help myself from the available video lecture by @rsnikhil, related documentation and the sail language manual. Actually @allenjbaum pointed me there. From what I have learnt so far, I did some sail support work i.e., the pull requests for Zb* , zmmul and zfh extensions support are under review. I am improving thanks to @jrtc27 (from her reviews). I am interested in helping with above work, documentation and Sail format etc.

jrtc27 · 2021-11-19T18:09:07Z

Oh I had already started trying to clean things up a bit more on Wednesday between meetings, finishing that off is something I had planned for the weekend, so assuming you/Tariq/Martin/Scott don't spot anything else that should be changed I imagine we should be able to get it merged next week.

Re removing the flag from X_or_F, reg_or_freg already implicitly checks whether Zfinx is enabled, so I think it's better to be consistent, as well as avoid all the repetition.

scottj97 · 2021-11-19T18:24:33Z

I am not too keen on changing X_or_F functions to remove the status flag from the arguments as that would make it less obvious what is happening !

The term "X_or_F" makes it abundantly clear what's happening. If someone cares to know how the choice is made between X or F they can look in that function.

Especially if passing sys_enable_zfinx() means that register assignment syntax is no longer supported, that's a terrible tradeoff.

abukharmeh · 2021-11-19T18:29:39Z

@Alasdair +1 for some kind of code formatter, it would help everyone trying to submit PRs to the model, and would reduce the amount of reviewing if the style is consistent across all PRs.

Suggestion for lint warning, it appears that the convention here to enforce spaces around commas, though I am not sure that's it make sense for the compiler to warn about such things. Perhaps we can have couple of such rules enabled by strict lint argument or something similar.

scottj97 suggested changes Nov 16, 2020

View reviewed changes

model/riscv_insts_fext.sail Outdated Show resolved Hide resolved

model/riscv_insts_fext.sail Outdated Show resolved Hide resolved

model/riscv_insts_fext.sail Outdated Show resolved Hide resolved

abukharmeh force-pushed the ZFinX_Implementation branch from 19c9ce8 to b28bfc2 Compare December 21, 2020 10:58

jrtc27 reviewed Feb 12, 2021

View reviewed changes

abukharmeh force-pushed the ZFinX_Implementation branch from 02b7d9c to 2d0c506 Compare October 18, 2021 12:51

Ibrahim Abu Kharmeh 84175316 added 6 commits October 26, 2021 13:36

Adds Zfinx enable flag

c0bc650

Hardwire misa.{f,d} and mstats.FS to 0

8120e35

Moving nan boxing functions to fdext_reg

5695f97

Swaps register names for floating point instructions

aaf1ac2

Adds new mapping to swap register names, and use it in all assembly clauses

Disable Floating point loads, stores and moves

f57e46d

Add X_or_F_s and X_or_F_d functions, and use them to access all regis…

24d0dc3

…ters for floating points Changes register accessed for floating point instructions and modify nan boxing functions for zfinx

abukharmeh force-pushed the ZFinX_Implementation branch from 2d0c506 to 24d0dc3 Compare October 26, 2021 17:44

Formatting

4148ed0

Remove couple of misplaced whitespace, unnecessary parens

abukharmeh requested a review from jrtc27 October 27, 2021 10:42

Fix inconsistent indentation in insts_dext file

8f027c4

martinberger merged commit c5e62ea into riscv:master Nov 17, 2021

jrtc27 added a commit that referenced this pull request Nov 17, 2021

Revert "Initial introduction of zfinx (#75)"

3f41ebe

This reverts commit c5e62ea.

jrtc27 mentioned this pull request Nov 17, 2021

Revert "Initial introduction of zfinx" #127

Merged

jrtc27 added a commit that referenced this pull request Nov 17, 2021

Revert "Initial introduction of zfinx (#75)"

af44432

This reverts commit c5e62ea.

scottj97 reviewed Nov 17, 2021

View reviewed changes

Initial introduction of zfinx #75

Initial introduction of zfinx #75

Conversation

abukharmeh commented Nov 16, 2020 • edited

scottj97 left a comment

Choose a reason for hiding this comment

pmundkur commented Nov 20, 2020 • edited

abukharmeh commented Nov 25, 2020 • edited

pmundkur commented Nov 25, 2020 • edited

abukharmeh commented Nov 25, 2020 • edited

pmundkur commented Nov 30, 2020 • edited

abukharmeh commented Dec 3, 2020 • edited

abukharmeh commented Dec 21, 2020 • edited

abukharmeh commented Feb 22, 2021

abukharmeh commented Oct 21, 2021

abukharmeh commented Oct 26, 2021 • edited

abukharmeh commented Oct 27, 2021

jrtc27 commented Oct 27, 2021

martinberger commented Oct 27, 2021 • edited

jrtc27 commented Oct 27, 2021

jrtc27 commented Oct 27, 2021

abukharmeh commented Oct 27, 2021

martinberger commented Nov 17, 2021 • edited

tariqkurd-repo commented Nov 17, 2021

jrtc27 commented Nov 17, 2021

jrtc27 commented Nov 17, 2021

tariqkurd-repo commented Nov 17, 2021 • edited

jrtc27 commented Nov 17, 2021 • edited

martinberger commented Nov 17, 2021

jrtc27 commented Nov 17, 2021

jrtc27 commented Nov 17, 2021

jrtc27 commented Nov 17, 2021

jrtc27 commented Nov 17, 2021

martinberger commented Nov 17, 2021

martinberger commented Nov 17, 2021

jrtc27 commented Nov 17, 2021

jrtc27 commented Nov 17, 2021

scottj97 commented Nov 17, 2021

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

abukharmeh Nov 17, 2021 • 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

jrtc27 commented Nov 17, 2021

Alasdair commented Nov 17, 2021

martinberger commented Nov 18, 2021

martinberger commented Nov 19, 2021 • edited

abukharmeh commented Nov 19, 2021 • edited

bilalsakhawat commented Nov 19, 2021

jrtc27 commented Nov 19, 2021 • edited

scottj97 commented Nov 19, 2021

abukharmeh commented Nov 19, 2021

abukharmeh commented Nov 16, 2020 •

edited

pmundkur commented Nov 20, 2020 •

edited

abukharmeh commented Nov 25, 2020 •

edited

pmundkur commented Nov 25, 2020 •

edited

abukharmeh commented Nov 25, 2020 •

edited

pmundkur commented Nov 30, 2020 •

edited

abukharmeh commented Dec 3, 2020 •

edited

abukharmeh commented Dec 21, 2020 •

edited

abukharmeh commented Oct 26, 2021 •

edited

martinberger commented Oct 27, 2021 •

edited

martinberger commented Nov 17, 2021 •

edited

tariqkurd-repo commented Nov 17, 2021 •

edited

jrtc27 commented Nov 17, 2021 •

edited

abukharmeh Nov 17, 2021 •

edited

martinberger commented Nov 19, 2021 •

edited

abukharmeh commented Nov 19, 2021 •

edited

jrtc27 commented Nov 19, 2021 •

edited