Re: [PATCH v14 02/26] target/loongarch: Add core definition

[WANG Xuerui]

Hi,

On 1/10/22 21:00, gaosong wrote:

Hi,

On 2022/1/9 下午5:25, WANG Xuerui wrote:

+
+const char * const fregnames[32] = {
+    "f0", "f1", "f2", "f3", "f4", "f5", "f6", "f7",
+    "f8", "f9", "f10", "f11", "f12", "f13", "f14", "f15",
+    "f16", "f17", "f18", "f19", "f20", "f21", "f22", "f23",
+    "f24", "f25", "f26", "f27", "f28", "f29", "f30", "f31",
+};
+
+static const char * const excp_names[EXCP_LAST + 1] = {
+    [EXCP_SYSCALL] = "Syscall",
+    [EXCP_BREAK] = "Break",
+    [EXCP_INE] = "Instruction Non-existent",

Nit: "Instruction Non-Existent" (or is there any authoritative source for this spelling? the English translation of the manual?)

I must admit that your English is really good, But 'Instruction Non-existent' is what you pointed out in v7.  I have a history [1], 

[1] https://patchew.org/QEMU/1634561247-25499-1-git-send-email-gaosong@loongson.cn/1634561247-25499-3-git-send-email-gaosong@loongson.cn/

> +static const char * const excp_names[EXCP_LAST + 1] = {
> +    [EXCP_ADE] = "Address error",
> +    [EXCP_SYSCALL] = "Syscall",
> +    [EXCP_BREAK] = "Break",
> +    [EXCP_INE] = "Inst. Not Exist",
Nit: "Instruction Non-existent", no need to shorten "instruction" like 
this IMO; no other similar usages exist so this would not be consistent.

In any case thank you for your other suggestions.

Well, I do make my mistakes sometimes, and I'll explain a bit: in the former review what I noticed was the broken English, and I pointed that out, but meanwhile I've forgotten I had done that, and why I didn't write "Non-Existent" was because the "Address error" wasn't in full Title Case. However, in this revision, what I noticed is the inconsistent casing (with EXCP_ADE removed, all other strings are in Title Case now), hence the reply.

No need to go full self-defense over this; I think what matters after all is just consistency.

> Thanks
> Song
>