Urgent build problem with Re: patch to allow any character in keyword

Urgent build problem with Re: patch to allow any character in keyword

[Frank Ch. Eigler]

geoffk wrote:

: ===File ~/patches/cgen-keywordspecials.patch================
: Index: devo/opcodes/ChangeLog
: 2001-06-13  Geoffrey Keating  <geoffk@redhat.com>
: 
: 	* cgen-asm.c (cgen_parse_keyword): When looking for the
: 	boundaries of a keyword, allow any special characters
: 	that are actually in one of the allowed keyword.
: 	* cgen-opc.c (cgen_keyword_add): Add any special characters
: 	to the nonalpha_chars field.
: 
: Index: devo/cgen/ChangeLog
: 2001-06-13  Geoffrey Keating  <geoffk@redhat.com>
: 
: 	* desc.scm (<keyword> 'gen-defn): Add extra zero into
: 	CGEN_KEYWORD_ENTRY initializers.
: 
: Index: devo/include/opcode/ChangeLog
: 2001-06-13  Geoffrey Keating  <geoffk@redhat.com>
: 
: 	* cgen.h (cgen_keyword): Add nonalpha_chars field.


Oops, there is a big problem with this patch, and I think it will have
to be reverted for now.

This is because the cgen.h change is not compatible with previously
generated opcodes files.  That in turn requires that every single
existing cgen-based opcodes port has to be regenerated (and given a
test run, etc.)  at the same time.  This hasn't been done, and
therefore, builds of older ports now break.

So, Geoff, want to revert or regenerate/retest them all?

I'm sorry I didn't realize this yesterday.  This hassle suggests that
a solution that doesn't involve changing global structs should be
investigated.


- FChE

Re: Urgent build problem with Re: patch to allow any character in keyword

[Geoff Keating]

> Cc: cgen@sources.redhat.com
> From: fche@redhat.com (Frank Ch. Eigler)
> Date: 15 Jun 2001 13:16:11 -0400
> User-Agent: Gnus/5.0808 (Gnus v5.8.8) XEmacs/21.1 (Cuyahoga Valley)
> 
> 
> geoffk wrote:
> 
> : ===File ~/patches/cgen-keywordspecials.patch================
> : Index: devo/opcodes/ChangeLog
> : 2001-06-13  Geoffrey Keating  <geoffk@redhat.com>
> : 
> : 	* cgen-asm.c (cgen_parse_keyword): When looking for the
> : 	boundaries of a keyword, allow any special characters
> : 	that are actually in one of the allowed keyword.
> : 	* cgen-opc.c (cgen_keyword_add): Add any special characters
> : 	to the nonalpha_chars field.
> : 
> : Index: devo/cgen/ChangeLog
> : 2001-06-13  Geoffrey Keating  <geoffk@redhat.com>
> : 
> : 	* desc.scm (<keyword> 'gen-defn): Add extra zero into
> : 	CGEN_KEYWORD_ENTRY initializers.
> : 
> : Index: devo/include/opcode/ChangeLog
> : 2001-06-13  Geoffrey Keating  <geoffk@redhat.com>
> : 
> : 	* cgen.h (cgen_keyword): Add nonalpha_chars field.
> 
> 
> Oops, there is a big problem with this patch, and I think it will have
> to be reverted for now.
> 
> This is because the cgen.h change is not compatible with previously
> generated opcodes files.  That in turn requires that every single
> existing cgen-based opcodes port has to be regenerated (and given a
> test run, etc.)  at the same time.  This hasn't been done, and
> therefore, builds of older ports now break.
> 
> So, Geoff, want to revert or regenerate/retest them all?
> 
> I'm sorry I didn't realize this yesterday.  This hassle suggests that
> a solution that doesn't involve changing global structs should be
> investigated.

Because the field is only added, at the end, and the default
initialisation is OK, it's not necessary to regenerate the opcode
files.  If the warning is bugging people, I can easily hand-edit all
the opcode files to add the extra "".

-- 
- Geoffrey Keating <geoffk@geoffk.org>

Re: Urgent build problem with Re: patch to allow any character in keyword

[J. Johnston]

Geoff Keating wrote:
> 
> > Cc: cgen@sources.redhat.com
> > From: fche@redhat.com (Frank Ch. Eigler)
> > Date: 15 Jun 2001 13:16:11 -0400
> > User-Agent: Gnus/5.0808 (Gnus v5.8.8) XEmacs/21.1 (Cuyahoga Valley)
> >
> >
> > geoffk wrote:
> >
> > : ===File ~/patches/cgen-keywordspecials.patch================
> > : Index: devo/opcodes/ChangeLog
> > : 2001-06-13  Geoffrey Keating  <geoffk@redhat.com>
> > :
> > :     * cgen-asm.c (cgen_parse_keyword): When looking for the
> > :     boundaries of a keyword, allow any special characters
> > :     that are actually in one of the allowed keyword.
> > :     * cgen-opc.c (cgen_keyword_add): Add any special characters
> > :     to the nonalpha_chars field.
> > :
> > : Index: devo/cgen/ChangeLog
> > : 2001-06-13  Geoffrey Keating  <geoffk@redhat.com>
> > :
> > :     * desc.scm (<keyword> 'gen-defn): Add extra zero into
> > :     CGEN_KEYWORD_ENTRY initializers.
> > :
> > : Index: devo/include/opcode/ChangeLog
> > : 2001-06-13  Geoffrey Keating  <geoffk@redhat.com>
> > :
> > :     * cgen.h (cgen_keyword): Add nonalpha_chars field.
> >
> >
> > Oops, there is a big problem with this patch, and I think it will have
> > to be reverted for now.
> >
> > This is because the cgen.h change is not compatible with previously
> > generated opcodes files.  That in turn requires that every single
> > existing cgen-based opcodes port has to be regenerated (and given a
> > test run, etc.)  at the same time.  This hasn't been done, and
> > therefore, builds of older ports now break.
> >
> > So, Geoff, want to revert or regenerate/retest them all?
> >
> > I'm sorry I didn't realize this yesterday.  This hassle suggests that
> > a solution that doesn't involve changing global structs should be
> > investigated.
> 
> Because the field is only added, at the end, and the default
> initialisation is OK, it's not necessary to regenerate the opcode
> files.  If the warning is bugging people, I can easily hand-edit all
> the opcode files to add the extra "".
> 

The problem Frank is referring to is with an internal port which has an instruction that has a
period to specify optional modes of the insn.  Although I cannot discuss the exact instruction, I
can provide a simplified example of the problem. Let's say we have:

    insn.XY.AB #imm

where XY is one of [x y] 
and AB is one of [a b] 

Each mode translates directly into a single bit in the encoded instruction.  The assembler allows
the user to omit both XY and AB or just AB.  In those cases, the settings are defaulted.

The relevant CGEN coding for the .x/.y and .a/.b options plus defaults are coded as follows:

 (define-hardware
  (name h-XY)
  (comment "XY selection bit")
  (attrs)
  (type immediate (UINT 1))
  (values keyword "" (("" 1) (".x" 0) (".y" 1)))
)

(define-hardware
  (name h-AB)
  (comment "AB selection bit")
  (attrs)
  (type immediate (UINT 1))
  (values keyword "" (("" 1) (".a" 0) (".b" 1)))
)

(dnop XY       "XY select bits"   () h-XY       f-XY)
(dnop AB       "AB select bits"   () h-AB       f-AB)

With the latest change, the assembler builds, however a regression occurs trying to assemble:

  insn.x.a  #3  

The assembly fails with:

Error: syntax error (expected char ` ', found `.') `insn.x.a #3'

For the particular internal platform, this error stops the compiler from building.  I know of at
least one other internal platform that uses this technique to encode settings specified at the end
of the instruction mnemonic using a period separator.

I found that backing out the changes to: include/opcode/cgen.h, opcodes/cgen-asm.c, and
opcodes/cgen-opc.c fixed the regression.  It may be possible that only one or two of the files had
to be changed, but I did not attempt to test this.

-- Jeff J.

Re: Urgent build problem with Re: patch to allow any character in keyword

[Geoff Keating]

Aah, I see.  The original keyword matcher looked for a sequence of the
form

.[a-zA-Z0-9_]*

The new one looks at what nonalpha characters are actually in
keywords ':chars:', and then tries to match

[:chars:a-zA-Z0-9]*

We could try changing this to

.[:chars:a-zA-Z0-9]*

which would probably fix your problem.  Of course, it'd be better if
there was a real lexer for this sort of thing.

-- 
- Geoffrey Keating <geoffk@geoffk.org>

Re: Urgent build problem with Re: patch to allow any character in keyword

[Geoff Keating]

Does this patch fix the problem?  I've checked that it still works
with my port.

-- 
- Geoffrey Keating <geoffk@geoffk.org>

===File ~/patches/cygnus/cgen-keywordfirstspecial.patch=====
2001-06-19  Geoffrey Keating  <geoffk@redhat.com>

	* cgen-asm.c (cgen_parse_keyword): Allow any first character.
	* cgen-opc.c (cgen_keyword_add): Ignore special first
	character when building nonalpha_chars field.

Index: opcodes/cgen-asm.c
===================================================================
RCS file: /cvs/cvsfiles/devo/opcodes/cgen-asm.c,v
retrieving revision 1.21
diff -p -u -p -r1.21 cgen-asm.c
--- cgen-asm.c	2001/06/14 21:00:28	1.21
+++ cgen-asm.c	2001/06/19 07:55:33
@@ -212,6 +212,12 @@ cgen_parse_keyword (cd, strp, keyword_ta
 
   p = start = *strp;
 
+  /* Allow any first character.  This is to make life easier for
+     the fairly common case of suffixes, eg. 'ld.b.w', where the first
+     character of the suffix ('.') is special.  */
+  if (*p)
+    ++p;
+  
   /* Allow letters, digits, and any special characters.  */
   while (((p - start) < (int) sizeof (buf))
 	 && *p
Index: opcodes/cgen-opc.c
===================================================================
RCS file: /cvs/cvsfiles/devo/opcodes/cgen-opc.c,v
retrieving revision 1.28
diff -p -u -p -r1.28 cgen-opc.c
--- cgen-opc.c	2001/06/14 21:11:59	1.28
+++ cgen-opc.c	2001/06/19 07:55:33
@@ -134,7 +134,7 @@ cgen_keyword_add (kt, ke)
   if (ke->name[0] == 0)
     kt->null_entry = ke;
 
-  for (i = 0; i < strlen (ke->name); i++)
+  for (i = 1; i < strlen (ke->name); i++)
     if (! isalnum ((unsigned char) ke->name[i])
 	&& ! strchr (kt->nonalpha_chars, ke->name[i]))
       {
============================================================

Re: Urgent build problem with Re: patch to allow any character in keyword

[J. Johnston]

Geoff Keating wrote:
> 
> Does this patch fix the problem?  I've checked that it still works
> with my port.
> 

It fixes the original problem, however, it causes a new failure with mnemonics that use ".".  For
example, insn.1 vs insn.2.  In the internal port, these two insns are distinct - they use different
operands.  The original scenario I reported differed because the operands were the same.

-- Jeff J.


> --
> - Geoffrey Keating <geoffk@geoffk.org>
> 
> ===File ~/patches/cygnus/cgen-keywordfirstspecial.patch=====
> 2001-06-19  Geoffrey Keating  <geoffk@redhat.com>
> 
>         * cgen-asm.c (cgen_parse_keyword): Allow any first character.
>         * cgen-opc.c (cgen_keyword_add): Ignore special first
>         character when building nonalpha_chars field.
> 
> Index: opcodes/cgen-asm.c
> ===================================================================
> RCS file: /cvs/cvsfiles/devo/opcodes/cgen-asm.c,v
> retrieving revision 1.21
> diff -p -u -p -r1.21 cgen-asm.c
> --- cgen-asm.c  2001/06/14 21:00:28     1.21
> +++ cgen-asm.c  2001/06/19 07:55:33
> @@ -212,6 +212,12 @@ cgen_parse_keyword (cd, strp, keyword_ta
> 
>    p = start = *strp;
> 
> +  /* Allow any first character.  This is to make life easier for
> +     the fairly common case of suffixes, eg. 'ld.b.w', where the first
> +     character of the suffix ('.') is special.  */
> +  if (*p)
> +    ++p;
> +
>    /* Allow letters, digits, and any special characters.  */
>    while (((p - start) < (int) sizeof (buf))
>          && *p
> Index: opcodes/cgen-opc.c
> ===================================================================
> RCS file: /cvs/cvsfiles/devo/opcodes/cgen-opc.c,v
> retrieving revision 1.28
> diff -p -u -p -r1.28 cgen-opc.c
> --- cgen-opc.c  2001/06/14 21:11:59     1.28
> +++ cgen-opc.c  2001/06/19 07:55:33
> @@ -134,7 +134,7 @@ cgen_keyword_add (kt, ke)
>    if (ke->name[0] == 0)
>      kt->null_entry = ke;
> 
> -  for (i = 0; i < strlen (ke->name); i++)
> +  for (i = 1; i < strlen (ke->name); i++)
>      if (! isalnum ((unsigned char) ke->name[i])
>         && ! strchr (kt->nonalpha_chars, ke->name[i]))
>        {
> ============================================================