riscv-ctg icon indicating copy to clipboard operation
riscv-ctg copied to clipboard

Published 20 hours ago • verified publisher icon riscv-software-src

F macro changes

Open shatyahariharan opened this issue 3 years ago • 9 comments

Hi, Based on the discussion I have modified the sigupd_f macro. And the definition of the SREG and LREG will get changed based on the each F test. The macro working fine with RV32F and RV64F. I have attached the link below for the test coverage details and the signature file. Link

shatyahariharan avatar Nov 02 '21 19:11 shatyahariharan

I think these macros would be far more readable if you define a helper macro: .macro NEWOFF()
.if NARG(VA_ARGS) == 1;\ .set offset ARG1(VA_ARGS,0) .endif .endm

You'd get rid of a lot of pesky IFs inside those macros.

You could do something similar, and more readable if you defined something like: .macro STORE_SZ .set ST_FSIG sw .if XLEN>FLEN . set ST_FSIG SREG .endif .endm

On Thu, Nov 11, 2021 at 8:21 AM S Pawan Kumar @.***> wrote:

@.**** commented on this pull request.

In riscv_ctg/env/arch_test.h https://github.com/riscv-software-src/riscv-ctg/pull/20#discussion_r747637523 :

 .set offset,offset+(2*REGWIDTH);\

.endif; + +#define FLENN()

The FLENN macro is no longer needed.

In riscv_ctg/env/arch_test.h https://github.com/riscv-software-src/riscv-ctg/pull/20#discussion_r747638028 :

#define RVTEST_SIGUPD_FID(_BR,_R,_F,...)\

  • FLENN();\

The FLENN macro should not be used anymore.

In riscv_ctg/env/arch_test.h https://github.com/riscv-software-src/riscv-ctg/pull/20#discussion_r747639747 :

.if NARG(VA_ARGS) == 1;\

  • SREG _R,_ARG1(VA_ARGS,0)(_BR);\
  • SREG _F,_ARG1(VA_ARGS,0)+REGWIDTH(_BR);\
  • .if XLEN==64 && FLEN==32;\
  •  sw _R,_ARG1(__VA_ARGS__,0)(_BR);\

  •  sw _F,_ARG1(__VA_ARGS__,0)+REGWIDTH(_BR);\

Should not be +REGWIDTH. Should be +8.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/riscv-software-src/riscv-ctg/pull/20#pullrequestreview-803981421, or unsubscribe https://github.com/notifications/unsubscribe-auth/AHPXVJUKJAUZHE5J3R3K7JLULPURTANCNFSM5HHI4KLQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

allenjbaum avatar Nov 12 '21 09:11 allenjbaum

Is this a valid case of floating point being not equal to XLEN?

On Thu, Nov 11, 2021 at 12:04 PM shatyahariharan @.***> wrote:

@.**** commented on this pull request.

In riscv_ctg/env/arch_test.h https://github.com/riscv-software-src/riscv-ctg/pull/20#discussion_r747672539 :

.if NARG(VA_ARGS) == 1;\

  • SREG _R,_ARG1(VA_ARGS,0)(_BR);\
  • SREG _F,_ARG1(VA_ARGS,0)+REGWIDTH(_BR);\
  • .if XLEN==64 && FLEN==32;\
  •  sw _R,_ARG1(__VA_ARGS__,0)(_BR);\

  •  sw _F,_ARG1(__VA_ARGS__,0)+REGWIDTH(_BR);\

For XLEN==64 && FLEN==32 the regwidth should be 4 right?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/riscv-software-src/riscv-ctg/pull/20#discussion_r747672539, or unsubscribe https://github.com/notifications/unsubscribe-auth/AD72ZM3Q3LFFXSHGTWRLZ3DULPZQ3ANCNFSM5HHI4KLQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

-- /**************** Marc Karasek ASM T648 Herder of Cats ****************/

MarcKarasek avatar Nov 13 '21 02:11 MarcKarasek

I don't understand the question. RV64F is a case where they aren't equal. RV32D is another - so they're valid. I'm pretty sure that's not your point though. Could you expand on your question?

Are you specifically asking that in the case of XLEN=64, that an sw is being performed rather than SREG? IT's hard to tell what this macro intends because... no comments, and I've complained about this before. Every macro should have a comment preceding it to explain its intended use.

I would reject any any change to macros like this that don't have a comment line.,

On Fri, Nov 12, 2021 at 6:13 PM Marc Karasek @.***> wrote:

Is this a valid case of floating point being not equal to XLEN?

On Thu, Nov 11, 2021 at 12:04 PM shatyahariharan @.***> wrote:

@.**** commented on this pull request.

In riscv_ctg/env/arch_test.h < https://github.com/riscv-software-src/riscv-ctg/pull/20#discussion_r747672539

:

.if NARG(VA_ARGS) == 1;\

  • SREG _R,_ARG1(VA_ARGS,0)(_BR);\
  • SREG _F,_ARG1(VA_ARGS,0)+REGWIDTH(_BR);\
  • .if XLEN==64 && FLEN==32;\
  • sw _R,_ARG1(VA_ARGS,0)(_BR);\
  • sw _F,_ARG1(VA_ARGS,0)+REGWIDTH(_BR);\

For XLEN==64 && FLEN==32 the regwidth should be 4 right?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub < https://github.com/riscv-software-src/riscv-ctg/pull/20#discussion_r747672539 , or unsubscribe < https://github.com/notifications/unsubscribe-auth/AD72ZM3Q3LFFXSHGTWRLZ3DULPZQ3ANCNFSM5HHI4KLQ

. Triage notifications on the go with GitHub Mobile for iOS < https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675

or Android < https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub .

-- /**************** Marc Karasek ASM T648 Herder of Cats ****************/

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/riscv-software-src/riscv-ctg/pull/20#issuecomment-967762567, or unsubscribe https://github.com/notifications/unsubscribe-auth/AHPXVJRZS5XDHSAZ5IPKUJTULXCVTANCNFSM5HHI4KLQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

allenjbaum avatar Nov 13 '21 02:11 allenjbaum

Also: this is a copy of the arch_test.h that's in the riscv_arch_test repo, and that is about to have massive changes. I'm wondering if it makes sense to break up this file into trap handler specific code, and everything else.

On Fri, Nov 12, 2021 at 6:57 PM Allen Baum @.***> wrote:

I don't understand the question. RV64F is a case where they aren't equal. RV32D is another - so they're valid. I'm pretty sure that's not your point though. Could you expand on your question?

Are you specifically asking that in the case of XLEN=64, that an sw is being performed rather than SREG? IT's hard to tell what this macro intends because... no comments, and I've complained about this before. Every macro should have a comment preceding it to explain its intended use.

I would reject any any change to macros like this that don't have a comment line.,

On Fri, Nov 12, 2021 at 6:13 PM Marc Karasek @.***> wrote:

Is this a valid case of floating point being not equal to XLEN?

On Thu, Nov 11, 2021 at 12:04 PM shatyahariharan @.***> wrote:

@.**** commented on this pull request.

In riscv_ctg/env/arch_test.h < https://github.com/riscv-software-src/riscv-ctg/pull/20#discussion_r747672539

:

.if NARG(VA_ARGS) == 1;\

  • SREG _R,_ARG1(VA_ARGS,0)(_BR);\
  • SREG _F,_ARG1(VA_ARGS,0)+REGWIDTH(_BR);\
  • .if XLEN==64 && FLEN==32;\
  • sw _R,_ARG1(VA_ARGS,0)(_BR);\
  • sw _F,_ARG1(VA_ARGS,0)+REGWIDTH(_BR);\

For XLEN==64 && FLEN==32 the regwidth should be 4 right?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub < https://github.com/riscv-software-src/riscv-ctg/pull/20#discussion_r747672539 , or unsubscribe < https://github.com/notifications/unsubscribe-auth/AD72ZM3Q3LFFXSHGTWRLZ3DULPZQ3ANCNFSM5HHI4KLQ

. Triage notifications on the go with GitHub Mobile for iOS < https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675

or Android < https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub .

-- /**************** Marc Karasek ASM T648 Herder of Cats ****************/

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/riscv-software-src/riscv-ctg/pull/20#issuecomment-967762567, or unsubscribe https://github.com/notifications/unsubscribe-auth/AHPXVJRZS5XDHSAZ5IPKUJTULXCVTANCNFSM5HHI4KLQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

allenjbaum avatar Nov 13 '21 03:11 allenjbaum

Maybe I am missing something..

Is FLEN the length of the floating point registers?

XLEN is the len of the other registers?

How can a RV64 have registers that are other than 64 bits? How can RV32 have 64 bit registers?

On Fri, Nov 12, 2021 at 9:57 PM Allen Baum @.***> wrote:

I don't understand the question. RV64F is a case where they aren't equal. RV32D is another - so they're valid. I'm pretty sure that's not your point though. Could you expand on your question?

Are you specifically asking that in the case of XLEN=64, that an sw is being performed rather than SREG? IT's hard to tell what this macro intends because... no comments, and I've complained about this before. Every macro should have a comment preceding it to explain its intended use.

I would reject any any change to macros like this that don't have a comment line.,

On Fri, Nov 12, 2021 at 6:13 PM Marc Karasek @.***> wrote:

Is this a valid case of floating point being not equal to XLEN?

On Thu, Nov 11, 2021 at 12:04 PM shatyahariharan @.***> wrote:

@.**** commented on this pull request.

In riscv_ctg/env/arch_test.h <

https://github.com/riscv-software-src/riscv-ctg/pull/20#discussion_r747672539

:

.if NARG(VA_ARGS) == 1;\

  • SREG _R,_ARG1(VA_ARGS,0)(_BR);\
  • SREG _F,_ARG1(VA_ARGS,0)+REGWIDTH(_BR);\
  • .if XLEN==64 && FLEN==32;\
  • sw _R,_ARG1(VA_ARGS,0)(_BR);\
  • sw _F,_ARG1(VA_ARGS,0)+REGWIDTH(_BR);\

For XLEN==64 && FLEN==32 the regwidth should be 4 right?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <

https://github.com/riscv-software-src/riscv-ctg/pull/20#discussion_r747672539

, or unsubscribe <

https://github.com/notifications/unsubscribe-auth/AD72ZM3Q3LFFXSHGTWRLZ3DULPZQ3ANCNFSM5HHI4KLQ

. Triage notifications on the go with GitHub Mobile for iOS <

https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675

or Android <

https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub

.

-- /**************** Marc Karasek ASM T648 Herder of Cats ****************/

— You are receiving this because you commented. Reply to this email directly, view it on GitHub < https://github.com/riscv-software-src/riscv-ctg/pull/20#issuecomment-967762567 , or unsubscribe < https://github.com/notifications/unsubscribe-auth/AHPXVJRZS5XDHSAZ5IPKUJTULXCVTANCNFSM5HHI4KLQ

. Triage notifications on the go with GitHub Mobile for iOS < https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675

or Android < https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub .

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/riscv-software-src/riscv-ctg/pull/20#issuecomment-967768035, or unsubscribe https://github.com/notifications/unsubscribe-auth/AD72ZM6POUYTCNJ4RYV3T73ULXHZXANCNFSM5HHI4KLQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

-- /**************** Marc Karasek ASM T648 Herder of Cats ****************/

MarcKarasek avatar Nov 13 '21 05:11 MarcKarasek

An RV32 processor with 32bit Xregisters will have 64bit FP registers if the D extension is supported. In that case, FLEN is 64, XLEN is 32. You can still perform 32fp ops in those 64b registers, or use all 64 bits. When you load a 32b FP number into a 64b FP reg, the HW "NAN boxes" it. I That sets the upper 32bits to a value that makes it appear to be a NAN (all1 exponent). 32b operations will ignore those upper bits and force the destination Freg to Nan box as well, 64b operations will treat it as a NAN so it's detectable.

On Fri, Nov 12, 2021 at 9:22 PM Marc Karasek @.***> wrote:

Maybe I am missing something..

Is FLEN the length of the floating point registers?

XLEN is the len of the other registers?

How can a RV64 have registers that are other than 64 bits? How can RV32 have 64 bit registers?

On Fri, Nov 12, 2021 at 9:57 PM Allen Baum @.***> wrote:

I don't understand the question. RV64F is a case where they aren't equal. RV32D is another - so they're valid. I'm pretty sure that's not your point though. Could you expand on your question?

Are you specifically asking that in the case of XLEN=64, that an sw is being performed rather than SREG? IT's hard to tell what this macro intends because... no comments, and I've complained about this before. Every macro should have a comment preceding it to explain its intended use.

I would reject any any change to macros like this that don't have a comment line.,

On Fri, Nov 12, 2021 at 6:13 PM Marc Karasek @.***> wrote:

Is this a valid case of floating point being not equal to XLEN?

On Thu, Nov 11, 2021 at 12:04 PM shatyahariharan @.***> wrote:

@.**** commented on this pull request.

In riscv_ctg/env/arch_test.h <

https://github.com/riscv-software-src/riscv-ctg/pull/20#discussion_r747672539

:

.if NARG(VA_ARGS) == 1;\

  • SREG _R,_ARG1(VA_ARGS,0)(_BR);\
  • SREG _F,_ARG1(VA_ARGS,0)+REGWIDTH(_BR);\
  • .if XLEN==64 && FLEN==32;\
  • sw _R,_ARG1(VA_ARGS,0)(_BR);\
  • sw _F,_ARG1(VA_ARGS,0)+REGWIDTH(_BR);\

For XLEN==64 && FLEN==32 the regwidth should be 4 right?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <

https://github.com/riscv-software-src/riscv-ctg/pull/20#discussion_r747672539

, or unsubscribe <

https://github.com/notifications/unsubscribe-auth/AD72ZM3Q3LFFXSHGTWRLZ3DULPZQ3ANCNFSM5HHI4KLQ

. Triage notifications on the go with GitHub Mobile for iOS <

https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675

or Android <

https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub

.

-- /**************** Marc Karasek ASM T648 Herder of Cats ****************/

— You are receiving this because you commented. Reply to this email directly, view it on GitHub <

https://github.com/riscv-software-src/riscv-ctg/pull/20#issuecomment-967762567

, or unsubscribe <

https://github.com/notifications/unsubscribe-auth/AHPXVJRZS5XDHSAZ5IPKUJTULXCVTANCNFSM5HHI4KLQ

. Triage notifications on the go with GitHub Mobile for iOS <

https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675

or Android <

https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub

.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub < https://github.com/riscv-software-src/riscv-ctg/pull/20#issuecomment-967768035 , or unsubscribe < https://github.com/notifications/unsubscribe-auth/AD72ZM6POUYTCNJ4RYV3T73ULXHZXANCNFSM5HHI4KLQ

. Triage notifications on the go with GitHub Mobile for iOS < https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675

or Android < https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub .

-- /**************** Marc Karasek ASM T648 Herder of Cats ****************/

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/riscv-software-src/riscv-ctg/pull/20#issuecomment-967784891, or unsubscribe https://github.com/notifications/unsubscribe-auth/AHPXVJWMBHYCWPY5OI66BLDULXYXVANCNFSM5HHI4KLQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

allenjbaum avatar Nov 13 '21 08:11 allenjbaum

This sounds like someone had way to much time on their hands and overengineered things.

On Sat, Nov 13, 2021, 3:21 AM Allen Baum @.***> wrote:

An RV32 processor with 32bit Xregisters will have 64bit FP registers if the D extension is supported. In that case, FLEN is 64, XLEN is 32. You can still perform 32fp ops in those 64b registers, or use all 64 bits. When you load a 32b FP number into a 64b FP reg, the HW "NAN boxes" it. I That sets the upper 32bits to a value that makes it appear to be a NAN (all1 exponent). 32b operations will ignore those upper bits and force the destination Freg to Nan box as well, 64b operations will treat it as a NAN so it's detectable.

On Fri, Nov 12, 2021 at 9:22 PM Marc Karasek @.***> wrote:

Maybe I am missing something..

Is FLEN the length of the floating point registers?

XLEN is the len of the other registers?

How can a RV64 have registers that are other than 64 bits? How can RV32 have 64 bit registers?

On Fri, Nov 12, 2021 at 9:57 PM Allen Baum @.***> wrote:

I don't understand the question. RV64F is a case where they aren't equal. RV32D is another - so they're valid. I'm pretty sure that's not your point though. Could you expand on your question?

Are you specifically asking that in the case of XLEN=64, that an sw is being performed rather than SREG? IT's hard to tell what this macro intends because... no comments, and I've complained about this before. Every macro should have a comment preceding it to explain its intended use.

I would reject any any change to macros like this that don't have a comment line.,

On Fri, Nov 12, 2021 at 6:13 PM Marc Karasek @.***> wrote:

Is this a valid case of floating point being not equal to XLEN?

On Thu, Nov 11, 2021 at 12:04 PM shatyahariharan @.***> wrote:

@.**** commented on this pull request.

In riscv_ctg/env/arch_test.h <

https://github.com/riscv-software-src/riscv-ctg/pull/20#discussion_r747672539

:

.if NARG(VA_ARGS) == 1;\

  • SREG _R,_ARG1(VA_ARGS,0)(_BR);\
  • SREG _F,_ARG1(VA_ARGS,0)+REGWIDTH(_BR);\
  • .if XLEN==64 && FLEN==32;\
  • sw _R,_ARG1(VA_ARGS,0)(_BR);\
  • sw _F,_ARG1(VA_ARGS,0)+REGWIDTH(_BR);\

For XLEN==64 && FLEN==32 the regwidth should be 4 right?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <

https://github.com/riscv-software-src/riscv-ctg/pull/20#discussion_r747672539

, or unsubscribe <

https://github.com/notifications/unsubscribe-auth/AD72ZM3Q3LFFXSHGTWRLZ3DULPZQ3ANCNFSM5HHI4KLQ

. Triage notifications on the go with GitHub Mobile for iOS <

https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675

or Android <

https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub

.

-- /**************** Marc Karasek ASM T648 Herder of Cats ****************/

— You are receiving this because you commented. Reply to this email directly, view it on GitHub <

https://github.com/riscv-software-src/riscv-ctg/pull/20#issuecomment-967762567

, or unsubscribe <

https://github.com/notifications/unsubscribe-auth/AHPXVJRZS5XDHSAZ5IPKUJTULXCVTANCNFSM5HHI4KLQ

. Triage notifications on the go with GitHub Mobile for iOS <

https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675

or Android <

https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub

.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub <

https://github.com/riscv-software-src/riscv-ctg/pull/20#issuecomment-967768035

, or unsubscribe <

https://github.com/notifications/unsubscribe-auth/AD72ZM6POUYTCNJ4RYV3T73ULXHZXANCNFSM5HHI4KLQ

. Triage notifications on the go with GitHub Mobile for iOS <

https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675

or Android <

https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub

.

-- /**************** Marc Karasek ASM T648 Herder of Cats ****************/

— You are receiving this because you commented. Reply to this email directly, view it on GitHub < https://github.com/riscv-software-src/riscv-ctg/pull/20#issuecomment-967784891 , or unsubscribe < https://github.com/notifications/unsubscribe-auth/AHPXVJWMBHYCWPY5OI66BLDULXYXVANCNFSM5HHI4KLQ

. Triage notifications on the go with GitHub Mobile for iOS < https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675

or Android < https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub .

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/riscv-software-src/riscv-ctg/pull/20#issuecomment-967803976, or unsubscribe https://github.com/notifications/unsubscribe-auth/AD72ZM7CO7VWZZX24KGNHM3ULYN2NANCNFSM5HHI4KLQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

MarcKarasek avatar Nov 13 '21 13:11 MarcKarasek

My stake in the ground is that no macro that is part of the default set should be approved that doesn't have a comment explaining what it is intended to be use for. This doesn't, and it won't get merged into the arch-test repo unless it does.

allenjbaum avatar Nov 17 '21 16:11 allenjbaum

I agree with you. We really need to comment everything. So that in the future we or someone else will be able to tell what does what.

On Wed, Nov 17, 2021, 11:46 AM Allen Baum @.***> wrote:

My stake in the ground is that no macro that is part of the default set should be approved that doesn't have a comment explaining what it is intended to be use for. This doesn't, and it won't get merged into the arch-test repo unless it does.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/riscv-software-src/riscv-ctg/pull/20#issuecomment-971761806, or unsubscribe https://github.com/notifications/unsubscribe-auth/AD72ZM7R3YJQSBA4BZEV7ZLUMPL6XANCNFSM5HHI4KLQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

MarcKarasek avatar Nov 17 '21 17:11 MarcKarasek