| Message ID | c098153d-cbcc-1f93-29ef-661c5176d599@linux.ibm.com |
|---|---|
| State | New |
| Headers | show |
| Series | None | expand |
Ok, I understand better now. But if those macros are supposed to be replaced by hook functions, could you make that replacement part of the proposed patch? paul > On Jun 13, 2024, at 11:22 PM, Kewen.Lin <linkw@linux.ibm.com> wrote: > > Hi Paul, > > on 2024/6/14 04:07, Paul Koning wrote: >> What is the effect of this change? The original code intended to have "float" mean a 32 bit value, and "double" a 64 bit value. There aren't any larger floats, so I defined the long double size as 64 also. Is the right answer not to define it? > > Since sub-patch 09/52 will poison {FLOAT,DOUBLE,LONG_DOUBLE}_TYPE_SIZE, target code building will fail > if it still has these macros. As I'd like to squash these target changes onto 09/52, so I didn't note > the background/context here, sorry about that. > >> >> That part I understand, but why does the patch also remove FLOAT_TYPE_SIZE and DOUBLE_TYPE_SIZE without explanation and without mention in the changelog? > > Oops, thanks for catching! I just noticed this sub-patch has inconsistent subject & changelog, I should > have noticed this as it has a quite different subject from the others. :( With your finding, I just > re-visited all the other sub-patches, luckily they are consistent. > > The below is the updated revision, hope it looks good to you. Thanks again. > > BR, > Kewen > ----- > > Subject: [PATCH] pdp11: Remove macro {FLOAT,DOUBLE,LONG_DOUBLE}_TYPE_SIZE > > This is to remove macros {FLOAT,{,LONG_}DOUBLE}_TYPE_SIZE > defines in pdp11 port, as we want to replace these macros > with hook mode_for_floating_type and poison them. > > gcc/ChangeLog: > > * config/pdp11/pdp11.h (FLOAT_TYPE_SIZE): Remove. > (DOUBLE_TYPE_SIZE): Likewise. > (LONG_DOUBLE_TYPE_SIZE): Likewise. > --- > gcc/config/pdp11/pdp11.h | 11 ----------- > 1 file changed, 11 deletions(-) > > diff --git a/gcc/config/pdp11/pdp11.h b/gcc/config/pdp11/pdp11.h > index 2446fea0b58..6c8e045bc57 100644 > --- a/gcc/config/pdp11/pdp11.h > +++ b/gcc/config/pdp11/pdp11.h > @@ -71,17 +71,6 @@ along with GCC; see the file COPYING3. If not see > #define LONG_TYPE_SIZE 32 > #define LONG_LONG_TYPE_SIZE 64 > > -/* In earlier versions, FLOAT_TYPE_SIZE was selectable as 32 or 64, > - but that conflicts with Fortran language rules. Since there is no > - obvious reason why we should have that feature -- other targets > - generally don't have float and double the same size -- I've removed > - it. Note that it continues to be true (for now) that arithmetic is > - always done with 64-bit values, i.e., the FPU is always in "double" > - mode. */ > -#define FLOAT_TYPE_SIZE 32 > -#define DOUBLE_TYPE_SIZE 64 > -#define LONG_DOUBLE_TYPE_SIZE 64 > - > /* machine types from ansi */ > #define SIZE_TYPE "short unsigned int" /* definition of size_t */ > #define WCHAR_TYPE "short int" /* or long int???? */ > -- > 2.43.0 > >
Hi Paul, on 2024/6/14 23:20, Paul Koning wrote: > Ok, I understand better now. But if those macros are supposed to be replaced by hook functions, could you make that replacement part of the proposed patch? The default implementation of the introduced hook mode_for_floating_type returns SFmode for float and DFmode for double or long double, which matches what pdp11 port requires, so there is no need to add its own hook implementation. This patch series only re-define this hook macro with the customized hook implementation for those ports which need something beyond the default. BR, Kewen > > paul > >> On Jun 13, 2024, at 11:22 PM, Kewen.Lin <linkw@linux.ibm.com> wrote: >> >> Hi Paul, >> >> on 2024/6/14 04:07, Paul Koning wrote: >>> What is the effect of this change? The original code intended to have "float" mean a 32 bit value, and "double" a 64 bit value. There aren't any larger floats, so I defined the long double size as 64 also. Is the right answer not to define it? >> >> Since sub-patch 09/52 will poison {FLOAT,DOUBLE,LONG_DOUBLE}_TYPE_SIZE, target code building will fail >> if it still has these macros. As I'd like to squash these target changes onto 09/52, so I didn't note >> the background/context here, sorry about that. >> >>> >>> That part I understand, but why does the patch also remove FLOAT_TYPE_SIZE and DOUBLE_TYPE_SIZE without explanation and without mention in the changelog? >> >> Oops, thanks for catching! I just noticed this sub-patch has inconsistent subject & changelog, I should >> have noticed this as it has a quite different subject from the others. :( With your finding, I just >> re-visited all the other sub-patches, luckily they are consistent. >> >> The below is the updated revision, hope it looks good to you. Thanks again. >> >> BR, >> Kewen >> ----- >> >> Subject: [PATCH] pdp11: Remove macro {FLOAT,DOUBLE,LONG_DOUBLE}_TYPE_SIZE >> >> This is to remove macros {FLOAT,{,LONG_}DOUBLE}_TYPE_SIZE >> defines in pdp11 port, as we want to replace these macros >> with hook mode_for_floating_type and poison them. >> >> gcc/ChangeLog: >> >> * config/pdp11/pdp11.h (FLOAT_TYPE_SIZE): Remove. >> (DOUBLE_TYPE_SIZE): Likewise. >> (LONG_DOUBLE_TYPE_SIZE): Likewise. >> --- >> gcc/config/pdp11/pdp11.h | 11 ----------- >> 1 file changed, 11 deletions(-) >> >> diff --git a/gcc/config/pdp11/pdp11.h b/gcc/config/pdp11/pdp11.h >> index 2446fea0b58..6c8e045bc57 100644 >> --- a/gcc/config/pdp11/pdp11.h >> +++ b/gcc/config/pdp11/pdp11.h >> @@ -71,17 +71,6 @@ along with GCC; see the file COPYING3. If not see >> #define LONG_TYPE_SIZE 32 >> #define LONG_LONG_TYPE_SIZE 64 >> >> -/* In earlier versions, FLOAT_TYPE_SIZE was selectable as 32 or 64, >> - but that conflicts with Fortran language rules. Since there is no >> - obvious reason why we should have that feature -- other targets >> - generally don't have float and double the same size -- I've removed >> - it. Note that it continues to be true (for now) that arithmetic is >> - always done with 64-bit values, i.e., the FPU is always in "double" >> - mode. */ >> -#define FLOAT_TYPE_SIZE 32 >> -#define DOUBLE_TYPE_SIZE 64 >> -#define LONG_DOUBLE_TYPE_SIZE 64 >> - >> /* machine types from ansi */ >> #define SIZE_TYPE "short unsigned int" /* definition of size_t */ >> #define WCHAR_TYPE "short int" /* or long int???? */ >> -- >> 2.43.0 >> >> >
Thanks Kewen. Given that background, the patch is OK. paul > On Jun 16, 2024, at 10:01 PM, Kewen.Lin <linkw@linux.ibm.com> wrote: > > Hi Paul, > > on 2024/6/14 23:20, Paul Koning wrote: >> Ok, I understand better now. But if those macros are supposed to be replaced by hook functions, could you make that replacement part of the proposed patch? > > The default implementation of the introduced hook mode_for_floating_type > returns SFmode for float and DFmode for double or long double, which matches > what pdp11 port requires, so there is no need to add its own hook implementation. > This patch series only re-define this hook macro with the customized hook > implementation for those ports which need something beyond the default. > > BR, > Kewen > >> >> paul >> >>> On Jun 13, 2024, at 11:22 PM, Kewen.Lin <linkw@linux.ibm.com> wrote: >>> >>> Hi Paul, >>> >>> on 2024/6/14 04:07, Paul Koning wrote: >>>> What is the effect of this change? The original code intended to have "float" mean a 32 bit value, and "double" a 64 bit value. There aren't any larger floats, so I defined the long double size as 64 also. Is the right answer not to define it? >>> >>> Since sub-patch 09/52 will poison {FLOAT,DOUBLE,LONG_DOUBLE}_TYPE_SIZE, target code building will fail >>> if it still has these macros. As I'd like to squash these target changes onto 09/52, so I didn't note >>> the background/context here, sorry about that. >>> >>>> >>>> That part I understand, but why does the patch also remove FLOAT_TYPE_SIZE and DOUBLE_TYPE_SIZE without explanation and without mention in the changelog? >>> >>> Oops, thanks for catching! I just noticed this sub-patch has inconsistent subject & changelog, I should >>> have noticed this as it has a quite different subject from the others. :( With your finding, I just >>> re-visited all the other sub-patches, luckily they are consistent. >>> >>> The below is the updated revision, hope it looks good to you. Thanks again. >>> >>> BR, >>> Kewen >>> ----- >>> >>> Subject: [PATCH] pdp11: Remove macro {FLOAT,DOUBLE,LONG_DOUBLE}_TYPE_SIZE >>> >>> This is to remove macros {FLOAT,{,LONG_}DOUBLE}_TYPE_SIZE >>> defines in pdp11 port, as we want to replace these macros >>> with hook mode_for_floating_type and poison them. >>> >>> gcc/ChangeLog: >>> >>> * config/pdp11/pdp11.h (FLOAT_TYPE_SIZE): Remove. >>> (DOUBLE_TYPE_SIZE): Likewise. >>> (LONG_DOUBLE_TYPE_SIZE): Likewise. >>> --- >>> gcc/config/pdp11/pdp11.h | 11 ----------- >>> 1 file changed, 11 deletions(-) >>> >>> diff --git a/gcc/config/pdp11/pdp11.h b/gcc/config/pdp11/pdp11.h >>> index 2446fea0b58..6c8e045bc57 100644 >>> --- a/gcc/config/pdp11/pdp11.h >>> +++ b/gcc/config/pdp11/pdp11.h >>> @@ -71,17 +71,6 @@ along with GCC; see the file COPYING3. If not see >>> #define LONG_TYPE_SIZE 32 >>> #define LONG_LONG_TYPE_SIZE 64 >>> >>> -/* In earlier versions, FLOAT_TYPE_SIZE was selectable as 32 or 64, >>> - but that conflicts with Fortran language rules. Since there is no >>> - obvious reason why we should have that feature -- other targets >>> - generally don't have float and double the same size -- I've removed >>> - it. Note that it continues to be true (for now) that arithmetic is >>> - always done with 64-bit values, i.e., the FPU is always in "double" >>> - mode. */ >>> -#define FLOAT_TYPE_SIZE 32 >>> -#define DOUBLE_TYPE_SIZE 64 >>> -#define LONG_DOUBLE_TYPE_SIZE 64 >>> - >>> /* machine types from ansi */ >>> #define SIZE_TYPE "short unsigned int" /* definition of size_t */ >>> #define WCHAR_TYPE "short int" /* or long int???? */ >>> -- >>> 2.43.0 >>> >>> >> >
diff --git a/gcc/config/pdp11/pdp11.h b/gcc/config/pdp11/pdp11.h index 2446fea0b58..6c8e045bc57 100644 --- a/gcc/config/pdp11/pdp11.h +++ b/gcc/config/pdp11/pdp11.h @@ -71,17 +71,6 @@ along with GCC; see the file COPYING3. If not see #define LONG_TYPE_SIZE 32 #define LONG_LONG_TYPE_SIZE 64 -/* In earlier versions, FLOAT_TYPE_SIZE was selectable as 32 or 64, - but that conflicts with Fortran language rules. Since there is no - obvious reason why we should have that feature -- other targets - generally don't have float and double the same size -- I've removed - it. Note that it continues to be true (for now) that arithmetic is - always done with 64-bit values, i.e., the FPU is always in "double" - mode. */ -#define FLOAT_TYPE_SIZE 32 -#define DOUBLE_TYPE_SIZE 64 -#define LONG_DOUBLE_TYPE_SIZE 64 - /* machine types from ansi */ #define SIZE_TYPE "short unsigned int" /* definition of size_t */ #define WCHAR_TYPE "short int" /* or long int???? */