On Wed, May 9, 2012 at 1:46 AM, Sharad Singhai <sing...@google.com> wrote:
[...]
> +@item -fdump-rtl-all=stderr
> +@opindex fdump-rtl-all=stderr
You do not need to have a separate index entry for '=stderr' or '=stdout'.
Rather, expand the description to state this in all the documentation
for -fdump-xxx=yyy.
[...]
> +/* Return non-zero iff the USER_FILENAME corresponds to stdout or
> + stderr stream. */
> +
> +static int
> +dump_stream_p (const char *user_filename)
> +{
> + if (user_filename)
> + return !strncmp ("stderr", user_filename, 6) ||
> + !strncmp ("stdout", user_filename, 6);
> + else
> + return 0;
> +}
The name is ambiguous.
This function is testing whether its string argument designates one of
the *standard* output streams. Name it to reflect that..
Have it take the dump state context. Also the coding convention: the binary
operator "||" should be on next line. In fact the thing could be
simpler. Instead of
testing over and over again against "stderr" (once in this function, then again
later), just return the corresponding standard FILE* pointer.
Also, this is a case of overuse of strncmp. If you name the function
dump_get_standard_stream:
return strcmp("stderr", dfi->user_filename) == 0
? stderr
: stdcmp("stdout", dfi->use_filename)
? stdout
: NULL;
you can simplify:
> - name = get_dump_file_name (phase);
> dfi = get_dump_file_info (phase);
> - stream = fopen (name, dfi->state < 0 ? "w" : "a");
> - if (!stream)
> - error ("could not open dump file %qs: %m", name);
> + if (dump_stream_p (dfi->user_filename))
> + {
> + if (!strncmp ("stderr", dfi->user_filename, 6))
> + stream = stderr;
> + else
> + if (!strncmp ("stdout", dfi->user_filename, 6))
> + stream = stdout;
> + else
> + error ("unknown stream: %qs: %m", dfi->user_filename);
> + dfi->state = 1;
> + }
> else
> - dfi->state = 1;
> - free (name);
> + {
> + name = get_dump_file_name (phase);
> + stream = fopen (name, dfi->state < 0 ? "w" : "a");
> + if (!stream)
> + error ("could not open dump file %qs: %m", name);
> + else
> + dfi->state = 1;
> + free (name);
> + }
>
> if (flag_ptr)
> *flag_ptr = dfi->flags;
> @@ -987,35 +1017,45 @@ dump_flag_name (int phase)
> dump_begin. */
>
> void
> -dump_end (int phase ATTRIBUTE_UNUSED, FILE *stream)
> +dump_end (int phase, FILE *stream)
> {
> - fclose (stream);
> + struct dump_file_info *dfi = get_dump_file_info (phase);
> + if (!dump_stream_p (dfi->user_filename))
> + fclose (stream);
> }
>
> /* Enable all tree dumps. Return number of enabled tree dumps. */
>
> static int
> -dump_enable_all (int flags)
> +dump_enable_all (int flags, const char *user_filename)
> {
> int ir_dump_type = (flags & (TDF_TREE | TDF_RTL | TDF_IPA));
> int n = 0;
> size_t i;
>
> for (i = TDI_none + 1; i < (size_t) TDI_end; i++)
> - if ((dump_files[i].flags & ir_dump_type))
> - {
> - dump_files[i].state = -1;
> - dump_files[i].flags |= flags;
> - n++;
> - }
> + {
> + if ((dump_files[i].flags & ir_dump_type))
> + {
> + dump_files[i].state = -1;
> + dump_files[i].flags |= flags;
> + n++;
> + }
> + if (user_filename)
> + dump_files[i].user_filename = user_filename;
> + }
>
> for (i = 0; i < extra_dump_files_in_use; i++)
> - if ((extra_dump_files[i].flags & ir_dump_type))
> - {
> - extra_dump_files[i].state = -1;
> - extra_dump_files[i].flags |= flags;
> - n++;
> - }
> + {
> + if ((extra_dump_files[i].flags & ir_dump_type))
> + {
> + extra_dump_files[i].state = -1;
> + extra_dump_files[i].flags |= flags;
> + n++;
> + }
> + if (user_filename)
> + extra_dump_files[i].user_filename = user_filename;
> + }
>
> return n;
> }
> @@ -1037,7 +1077,7 @@ dump_switch_p_1 (const char *arg, struct dump_file
> if (!option_value)
> return 0;
>
> - if (*option_value && *option_value != '-')
> + if (*option_value && *option_value != '-' && *option_value != '=')
> return 0;
>
> ptr = option_value;
> @@ -1052,17 +1092,30 @@ dump_switch_p_1 (const char *arg, struct dump_file
> while (*ptr == '-')
> ptr++;
> end_ptr = strchr (ptr, '-');
> +
> if (!end_ptr)
> end_ptr = ptr + strlen (ptr);
> length = end_ptr - ptr;
>
> + if (*ptr == '=')
> + {
> + /* Interpret rest of the argument as a dump filename. The
> + user provided filename overrides generated dump names as
> + well as other command line filenames. */
> + flags |= TDF_USER_FILENAME;
> + if (dfi->user_filename)
> + free (dfi->user_filename);
> + dfi->user_filename = xstrdup (ptr + 1);
> + break;
> + }
> +
> for (option_ptr = dump_options; option_ptr->name; option_ptr++)
> if (strlen (option_ptr->name) == length
> && !memcmp (option_ptr->name, ptr, length))
> - {
> - flags |= option_ptr->value;
> + {
> + flags |= option_ptr->value;
> goto found;
> - }
> + }
> warning (0, "ignoring unknown option %q.*s in %<-fdump-%s%>",
> length, ptr, dfi->swtch);
> found:;
> @@ -1075,7 +1128,7 @@ dump_switch_p_1 (const char *arg, struct dump_file
> /* Process -fdump-tree-all and -fdump-rtl-all, by enabling all the
> known dumps. */
> if (dfi->suffix == NULL)
> - dump_enable_all (dfi->flags);
> + dump_enable_all (dfi->flags, dfi->user_filename);
>
> return 1;
> }
> @@ -1124,5 +1177,5 @@ dump_function (int phase, tree fn)
> bool
> enable_rtl_dump_file (void)
> {
> - return dump_enable_all (TDF_RTL | TDF_DETAILS | TDF_BLOCKS) > 0;
> + return dump_enable_all (TDF_RTL | TDF_DETAILS | TDF_BLOCKS, NULL) > 0;
> }
> Index: tree-pass.h
> ===================================================================
> --- tree-pass.h (revision 187265)
> +++ tree-pass.h (working copy)
> @@ -84,8 +84,9 @@ enum tree_dump_index
> #define TDF_ENUMERATE_LOCALS (1 << 22) /* Enumerate locals by uid. */
> #define TDF_CSELIB (1 << 23) /* Dump cselib details. */
> #define TDF_SCEV (1 << 24) /* Dump SCEV details. */
> +#define TDF_USER_FILENAME (1 << 25) /* Dump on user provided filename,
> + instead of constructing one. */
>
> -
> /* In tree-dump.c */
>
> extern char *get_dump_file_name (int);
> @@ -222,6 +223,8 @@ struct dump_file_info
> const char *suffix; /* suffix to give output file. */
> const char *swtch; /* command line switch */
> const char *glob; /* command line glob */
> + const char *user_filename; /* user provided filename instead of making
> + up one using dump_base_name + suffix. */
There is "no user" here: we are the users :-) Just call it "filename".
-- Gaby
