[6/9] ENABLE_CHECKING refactoring: generators
diff mbox

Message ID CAFiYyc2jujujKw2SYUur+wTD9LqL7SscZbGQmAbUZ5Dsu6ccaA@mail.gmail.com
State New
Headers show

Commit Message

Richard Biener Oct. 21, 2015, 10:49 a.m. UTC
On Mon, Oct 19, 2015 at 1:55 AM, Mikhail Maltsev <maltsevm@gmail.com> wrote:
> On 10/06/2015 03:56 PM, Richard Biener wrote:
>> The generators should simply unconditionally check (not in generated
>> files, of course).
>> And the generated code parts should use flag_checking.
>>
>> Richard.
>
> genautomata has some macros similar to tree checks, so I avoided changing them.
> genconditions for some reason #undef-s ENABLE_CHECKING in the generated code. I
> did not look at it in details, but decided to simply #define CHECKING_P to 0 for
> consistency.
>
> As for genextract and gengtype, I followed your recommendations, that is, used
> flag_checking instead of CHECKING_P in genextract, and always enable debugging
> functions in gengtype.

flag_checking will be never set, so as suggested make it do the checking bits
unconditionally.

Otherwise the patch is ok.  (needs sorting out the CHECKING_P vs.
ENABLE_CHECKING
elsewhere)

Thanks,
Richard.


> --
> Regards,
>     Mikhail Maltsev
>
> gcc/ChangeLog:
>
> 2015-10-19  Mikhail Maltsev  <maltsevm@gmail.com>
>
>         * genautomata.c: Use CHECKING_P instead of ENABLE_CHECKING.
>         * genconditions.c: Define CHECKING_P in the generated code.
>         * genextract.c: Use flag_checking in insn_extract.
>         * gengtype.c (main): Remove conditional compilation.
>         * gengtype.h: Likewise.

Comments

Jeff Law Oct. 28, 2015, 4:22 p.m. UTC | #1
On 10/21/2015 04:49 AM, Richard Biener wrote:
> On Mon, Oct 19, 2015 at 1:55 AM, Mikhail Maltsev <maltsevm@gmail.com> wrote:
>> On 10/06/2015 03:56 PM, Richard Biener wrote:
>>> The generators should simply unconditionally check (not in generated
>>> files, of course).
>>> And the generated code parts should use flag_checking.
>>>
>>> Richard.
>>
>> genautomata has some macros similar to tree checks, so I avoided changing them.
>> genconditions for some reason #undef-s ENABLE_CHECKING in the generated code. I
>> did not look at it in details, but decided to simply #define CHECKING_P to 0 for
>> consistency.
>>
>> As for genextract and gengtype, I followed your recommendations, that is, used
>> flag_checking instead of CHECKING_P in genextract, and always enable debugging
>> functions in gengtype.
>
> diff --git a/gcc/genextract.c b/gcc/genextract.c
> index fe97701..a03ac97 100644
> --- a/gcc/genextract.c
> +++ b/gcc/genextract.c
> @@ -373,10 +373,11 @@ insn_extract (rtx_insn *insn)\n{\n\
>     rtx pat = PATTERN (insn);\n\
>     int i ATTRIBUTE_UNUSED; /* only for peepholes */\n\
>   \n\
> -#ifdef ENABLE_CHECKING\n\
> -  memset (ro, 0xab, sizeof (*ro) * MAX_RECOG_OPERANDS);\n\
> -  memset (ro_loc, 0xab, sizeof (*ro_loc) * MAX_RECOG_OPERANDS);\n\
> -#endif\n");
> +  if (flag_checking)\n\
> +    {\n\
> +      memset (ro, 0xab, sizeof (*ro) * MAX_RECOG_OPERANDS);\n\
> +      memset (ro_loc, 0xab, sizeof (*ro_loc) * MAX_RECOG_OPERANDS);\n\
> +    }\n");
>
>
>
> flag_checking will be never set, so as suggested make it do the checking bits
> unconditionally.
It can be set as this code ends up in insn-extract.c.  That's the way it 
looked to me.  Just to be sure, I shoved in an abort() in that path and 
sure enough it fires as soon as we start trying to configure the stage1 
target libraries :-)

I've fixed a couple comments on #else/#endif lines.  I've never been a 
fan of those, but i can hold my nose while I make those comments consistent.

I'm doing a bootstrap & config-all.mk test and expect to commit this for 
Mikhail later today.

jeff

Patch
diff mbox

diff --git a/gcc/genextract.c b/gcc/genextract.c
index fe97701..a03ac97 100644
--- a/gcc/genextract.c
+++ b/gcc/genextract.c
@@ -373,10 +373,11 @@  insn_extract (rtx_insn *insn)\n{\n\
   rtx pat = PATTERN (insn);\n\
   int i ATTRIBUTE_UNUSED; /* only for peepholes */\n\
 \n\
-#ifdef ENABLE_CHECKING\n\
-  memset (ro, 0xab, sizeof (*ro) * MAX_RECOG_OPERANDS);\n\
-  memset (ro_loc, 0xab, sizeof (*ro_loc) * MAX_RECOG_OPERANDS);\n\
-#endif\n");
+  if (flag_checking)\n\
+    {\n\
+      memset (ro, 0xab, sizeof (*ro) * MAX_RECOG_OPERANDS);\n\
+      memset (ro_loc, 0xab, sizeof (*ro_loc) * MAX_RECOG_OPERANDS);\n\
+    }\n");