[05/57] rs6000: Add file support and functions for diagnostic support

Message ID e343b33930398e1baa20ab30cb1a74355f5b6636.1619537141.git.wschmidt@linux.ibm.com
State Superseded
Headers show
Series
  • Replace the Power target-specific built-in machinery
Related show

Commit Message

Kewen.Lin via Gcc-patches April 27, 2021, 3:32 p.m.
2021-03-03  Bill Schmidt  <wschmidt@linux.ibm.com>

gcc/
	* config/rs6000/rs6000-gen-builtins.c (bif_file): New filescope
	variable.
	(ovld_file): Likewise.
	(header_file): Likewise.
	(init_file): Likewise.
	(defines_file): Likewise.
	(pgm_path): Likewise.
	(bif_path): Likewise.
	(ovld_path): Likewise.
	(header_path): Likewise.
	(init_path): Likewise.
	(defines_path): Likewise.
	(LINELEN): New defined constant.
	(linebuf): New filescope variable.
	(line): Likewise.
	(pos): Likewise.
	(diag): Likewise.
	(bif_diag): New function.
	(ovld_diag): Likewise.
---
 gcc/config/rs6000/rs6000-gen-builtins.c | 47 +++++++++++++++++++++++++
 1 file changed, 47 insertions(+)

-- 
2.27.0

Comments

Segher Boessenkool May 20, 2021, 11:03 p.m. | #1
Hi!

On Tue, Apr 27, 2021 at 10:32:40AM -0500, Bill Schmidt via Gcc-patches wrote:
> 	* config/rs6000/rs6000-gen-builtins.c (bif_file): New filescope

> 	variable.


What makes it interesting that this var has file scope?  Did you mean to
say it has internal linkage ("is static")?  I would just leave that off
completely, anyway (just "New." or "New variable.")

> 	(LINELEN): New defined constant.


It isn't a constant, it's a macro.  "New macro."?

> +#define LINELEN 1024

> +static char linebuf[LINELEN];


You never get anywhere close to 1024 I suppose?

> +/* Pointer to a diagnostic function.  */

> +void (*diag) (const char *, ...) __attribute__ ((format (printf, 1, 2)))

> +  = NULL;


This isn't portable: you cannot assign NULL to a pointer to function.
But it will work on all POSIX machines, and those are the only hosts we
support.  Still icky :-)

If you just leave off the initialisation, it will be initialised to 0,
which is exactly the same problem of course, just less explicit.

This pointer is not static btw?  Should it be?


Okay for trunk with a little changelog massaging.  Thanks!


Segher
Kewen.Lin via Gcc-patches May 21, 2021, 1:06 p.m. | #2
On 5/20/21 6:03 PM, Segher Boessenkool wrote:
> Hi!

>

> On Tue, Apr 27, 2021 at 10:32:40AM -0500, Bill Schmidt via Gcc-patches wrote:

>> 	* config/rs6000/rs6000-gen-builtins.c (bif_file): New filescope

>> 	variable.

> What makes it interesting that this var has file scope?  Did you mean to

> say it has internal linkage ("is static")?  I would just leave that off

> completely, anyway (just "New." or "New variable.")

OK.  As you say, just pointing out these are all static linkage, outside 
all functions for convenience.  You'll find this in quite a number of 
patches in the series, so you can assume I'll fix this for all of them.
>

>> 	(LINELEN): New defined constant.

> It isn't a constant, it's a macro.  "New macro."?

Hey, these are old-school named constants!  :-)  (No problem, will 
change.  Again, you will see a lot of this in the change logs.)
>

>> +#define LINELEN 1024

>> +static char linebuf[LINELEN];

> You never get anywhere close to 1024 I suppose?


Correct; I'd be surprised if we get to a tenth of that on any line.

>> +/* Pointer to a diagnostic function.  */

>> +void (*diag) (const char *, ...) __attribute__ ((format (printf, 1, 2)))

>> +  = NULL;

> This isn't portable: you cannot assign NULL to a pointer to function.

> But it will work on all POSIX machines, and those are the only hosts we

> support.  Still icky :-)

>

> If you just leave off the initialisation, it will be initialised to 0,

> which is exactly the same problem of course, just less explicit.

Will do.
>

> This pointer is not static btw?  Should it be?

Yes, looks like it should...
> Okay for trunk with a little changelog massaging.  Thanks!


Thank you!

BIll


> Segher

Patch

diff --git a/gcc/config/rs6000/rs6000-gen-builtins.c b/gcc/config/rs6000/rs6000-gen-builtins.c
index 0afbff8e3ab..0e8b315208b 100644
--- a/gcc/config/rs6000/rs6000-gen-builtins.c
+++ b/gcc/config/rs6000/rs6000-gen-builtins.c
@@ -163,3 +163,50 @@  along with GCC; see the file COPYING3.  If not see
 #include <string.h>
 #include <assert.h>
 #include <unistd.h>
+
+/* Input and output file descriptors and pathnames.  */
+static FILE *bif_file;
+static FILE *ovld_file;
+static FILE *header_file;
+static FILE *init_file;
+static FILE *defines_file;
+
+static const char *pgm_path;
+static const char *bif_path;
+static const char *ovld_path;
+static const char *header_path;
+static const char *init_path;
+static const char *defines_path;
+
+/* Position information.  Note that "pos" is zero-indexed, but users
+   expect one-indexed column information, so representations of "pos"
+   as columns in diagnostic messages must be adjusted.  */
+#define LINELEN 1024
+static char linebuf[LINELEN];
+static int line;
+static int pos;
+
+/* Pointer to a diagnostic function.  */
+void (*diag) (const char *, ...) __attribute__ ((format (printf, 1, 2)))
+  = NULL;
+
+/* Custom diagnostics.  */
+static void __attribute__ ((format (printf, 1, 2)))
+bif_diag (const char * fmt, ...)
+{
+  va_list args;
+  fprintf (stderr, "%s:%d: ", bif_path, line);
+  va_start (args, fmt);
+  vfprintf (stderr, fmt, args);
+  va_end (args);
+}
+
+static void __attribute__ ((format (printf, 1, 2)))
+ovld_diag (const char * fmt, ...)
+{
+  va_list args;
+  fprintf (stderr, "%s:%d: ", ovld_path, line);
+  va_start (args, fmt);
+  vfprintf (stderr, fmt, args);
+  va_end (args);
+}