[PATCH] caret diagnostics (was: broken FE diagnostics wrt complex expressions)
2008/8/14 Tom Tromey <[EMAIL PROTECTED]>:
>
> ISTR Manuel having a patch for caret diagnostic output... ?
>
I was planning to submit it this week to consider it for GCC 4.4
(disabled by default). I am still testing it. Bootstrap and regression
testing with --enable-languages=all,ada,obj-c++ is very slow even on a
x86_64-unknown-gnu-linux-pc. But here it is for your reviewing and
testing pleasure. ;-)
The approach followed is to never free the input buffers and keep
pointers to the start of each line tracked by each line-map.
Alternative approaches are:
b) Re-open the file and fseek but that is not trivial since we need to
do it fast but still do all character conversions that we did when
libcpp opened it the first time. This is approximately what clang does
as far as I know except that they keep a cache of buffers ever
reopened. I think that thanks to our line-maps implementation, we can
do the seeking quite more efficiently in terms of computation time.
Still I do not know how to *properly* and *efficiently* re-open a
file.
c) Memcpy the relevant lines to a buffer for each line_map. This does
not seem a good approach under any circumstance. It means more memory
needed while the input buffers are opened (because we will end up with
two copies of the buffer) and we cannot free up that memory later
because we do not track how many references there are to each
line_maps, so we never free them. Perhaps the GC can help here? This
is the approach followed by GFortran.
Comments are welcome. However, I don't have time to do
memory/computation time tests (setting up the environment itself seems
very time consuming), so those would be *greatly* appreciated!
You can see many examples of the caret output by configuring with
--enable-caret-diagnostics, then reverting the changes to
gcc/testsuite/lib/gcc.exp and running the testsuite. Check the output
in the gcc.log and g++.log files.
Cheers,
Manuel.
Index: gcc/java/jcf-parse.c
===
--- gcc/java/jcf-parse.c(revision 138935)
+++ gcc/java/jcf-parse.c(working copy)
@@ -1193,12 +1193,14 @@ give_name_to_class (JCF *jcf, int i)
JPOOL_UTF_LENGTH (jcf, j));
this_class = lookup_class (class_name);
{
tree source_name = identifier_subst (class_name, "", '.', '/', ".java");
const char *sfname = find_sourcefile (IDENTIFIER_POINTER (source_name));
- linemap_add (line_table, LC_ENTER, false, sfname, 0);
- input_location = linemap_line_start (line_table, 0, 1);
+ /* FIXME CARET: We should add a pointer to the input line
+instead of NULL. */
+ linemap_add (line_table, LC_ENTER, false, sfname, 0, NULL);
+ input_location = linemap_line_start (line_table, 0, 1, NULL);
file_start_location = input_location;
DECL_SOURCE_LOCATION (TYPE_NAME (this_class)) = input_location;
if (main_input_filename == NULL && jcf == main_jcf)
main_input_filename = sfname;
}
@@ -1472,11 +1474,11 @@ jcf_parse (JCF* jcf)
fatal_error ("error while parsing final attributes");
if (TYPE_REFLECTION_DATA (current_class))
annotation_write_byte (JV_DONE_ATTR);
- linemap_add (line_table, LC_LEAVE, false, NULL, 0);
+ linemap_add (line_table, LC_LEAVE, false, NULL, 0, NULL);
/* The fields of class_type_node are already in correct order. */
if (current_class != class_type_node && current_class != object_type_node)
TYPE_FIELDS (current_class) = nreverse (TYPE_FIELDS (current_class));
@@ -1505,12 +1507,12 @@ load_inner_classes (tree cur_class)
static void
duplicate_class_warning (const char *filename)
{
location_t warn_loc;
- linemap_add (line_table, LC_RENAME, 0, filename, 0);
- warn_loc = linemap_line_start (line_table, 0, 1);
+ linemap_add (line_table, LC_RENAME, 0, filename, 0, NULL);
+ warn_loc = linemap_line_start (line_table, 0, 1, NULL);
warning (0, "%Hduplicate class will only be compiled once", &warn_loc);
}
static void
java_layout_seen_class_methods (void)
@@ -1558,11 +1560,11 @@ parse_class_file (void)
input_location = DECL_SOURCE_LOCATION (TYPE_NAME (current_class));
{
/* Re-enter the current file. */
expanded_location loc = expand_location (input_location);
-linemap_add (line_table, LC_ENTER, 0, loc.file, loc.line);
+linemap_add (line_table, LC_ENTER, 0, loc.file, loc.line, NULL);
}
file_start_location = input_location;
(*debug_hooks->start_source_file) (input_line, input_filename);
java_mark_class_local (current_class);
@@ -1625,11 +1627,11 @@ parse_class_file (void)
* Needs to be set before init_function_start. */
if (min_line == 0 || line < min_line)
min_line = line;
}
if (min_line != 0)
- input_location = linemap_line_start (line_table, min_line, 1);
+ input_location = linemap_line_start (line_table, min_li
Re: broken FE diagnostics wrt complex expressions
> Aldy> 1. beginning/ending locations functionality as Joseph suggests. > Aldy> 2. make sure the parsers pick the proper token/location. > Aldy> 3. error reporting machinery > > Aldy> How does this sound? > > It sounds good to me. #1 might be hard, I have not looked into it. Well, we can always start with that which produces the most bang for the buck, #2. I'd rather have a caret at the beginning of the error, than no caret at all. Then we can expand to beginning and end. > ISTR Manuel having a patch for caret diagnostic output... ? Is this manu at gcc.gnu.org? If so, copying him. Aldy
Re: broken FE diagnostics wrt complex expressions
2008/8/13 Aldy Hernandez <[EMAIL PROTECTED]>: > > 1. beginning/ending locations functionality as Joseph suggests. > 2. make sure the parsers pick the proper token/location. > 3. error reporting machinery There are various issues that would need to be addressed to have decent caret diagnostics: 1) We only track one location for each token, so things that are macro-expanded are difficult to get right (in some cases we want the original location and in other occasions we want the macro call location. In many cases we want both). Ideally we could encode this information in a single location_t object. 2) During parsing, token locations are underused. Ideally, every diagnostic should use a explicit location (input_location must die). Also, most build_something calls should take location info. 3) After parsing, few objects have location (because the build_something functions do not typically track it). Ideally for a binary operator we would be able to know the location of the operator and its operands, so we can print something like (as clang does): error: invalid operands to binary operator '+' ((someA.X + 40) + some.A) ^ 4) Find a way to move from a location_t to a null-terminated const char * as efficiently as possible. The patch I just submitted uses the most straightforward approach but improvements should be possible. 5) Make libcpp use the diagnostics.[ch] machinery. This will simplify many things, and avoid code duplication. Most of these things can be tackled independently. I am slowly tackling 2) and 4). Cheers, Manuel.
Re: broken FE diagnostics wrt complex expressions
2008/8/14 Aldy Hernandez <[EMAIL PROTECTED]>: >> Aldy> 1. beginning/ending locations functionality as Joseph suggests. >> Aldy> 2. make sure the parsers pick the proper token/location. >> Aldy> 3. error reporting machinery >> >> Aldy> How does this sound? >> >> It sounds good to me. #1 might be hard, I have not looked into it. > > Well, we can always start with that which produces the most bang for the > buck, #2. I'd rather have a caret at the beginning of the error, than > no caret at all. Then we can expand to beginning and end. > >> ISTR Manuel having a patch for caret diagnostic output... ? > > Is this manu at gcc.gnu.org? If so, copying him. I just send my current patch. http://gcc.gnu.org/ml/gcc/2008-08/msg00193.html I was planning to submit it formally later this week but you were faster than the GCC Farm can test my patch. Cheers, Manuel.
Re: broken FE diagnostics wrt complex expressions
2008/8/13 Tom Tromey <[EMAIL PROTECTED]>: >> "Tom" == Tom Tromey <[EMAIL PROTECTED]> writes: > > Tom> As far as I know nobody is actively working on any of this, though > Tom> Mañuel and I talk about it sporadically. > > Crap, I misspelled his name while trying extra to get it right. > Sorry about that. > XD. That is fine. I prefer that you make the error using the spanish 'ñ' than to call me "Manual" http://en.wikipedia.org/wiki/Manual http://en.wikipedia.org/wiki/Manuel Cheers, Manuel. PS: Now even the wikipedia gives a warning: "Manual: Not to be confused with Manuel." Funny.
Re: [PATCH] caret diagnostics (was: broken FE diagnostics wrt complex expressions)
2008/8/14 Joseph S. Myers <[EMAIL PROTECTED]>: > On Thu, 14 Aug 2008, Manuel López-Ibáñez wrote: > >> You can see many examples of the caret output by configuring with >> --enable-caret-diagnostics, then reverting the changes to >> gcc/testsuite/lib/gcc.exp and running the testsuite. Check the output >> in the gcc.log and g++.log files. > > It's clear it should be controlled by a -Wcaret (or similar) option rather > than a configure time option; the choice of style is a matter of user > preference. > It is controlled by -fdiagnostics-show-caret. See the diff for gcc/opts.c. The configure options are meant to enable/disable all code related to caret printing in a similar way as it was done with mapped locations. This was requested the first time I sent this patch because it was considered too experimental to have it even with -fno-diagnostics-show-caret as the default. Cheers, Manuel.
Re: broken FE diagnostics wrt complex expressions
On Wed, 13 Aug 2008, Aldy Hernandez wrote: > 1. beginning/ending locations functionality as Joseph suggests. Note that the GNU Coding Standards specify formats for diagnostics giving a range of locations; when GCC tracks such a range, it should use those formats (by default). source-file-name:lineno-1.column-1-lineno-2.column-2: message source-file-name:lineno-1.column-1-column-2: message source-file-name:lineno-1-lineno-2: message -- Joseph S. Myers [EMAIL PROTECTED]
Re: [PATCH] caret diagnostics (was: broken FE diagnostics wrt complex expressions)
On Thu, 14 Aug 2008, Manuel López-Ibáñez wrote:
> It is controlled by -fdiagnostics-show-caret. See the diff for gcc/opts.c.
>
> The configure options are meant to enable/disable all code related to
> caret printing in a similar way as it was done with mapped locations.
> This was requested the first time I sent this patch because it was
> considered too experimental to have it even with
> -fno-diagnostics-show-caret as the default.
Mapped locations were sitting around on trunk for a long time as a
bitrotten feature; I don't think that's a good example here. When the
code works and passes review it should go on trunk, default remaining the
present diagnostic style according to the GNU Coding Standards, no
configure option.
I don't think the option should necessarily just be boolean; once choice
that may make sense would be caret diagnostics for the first diagnostic
from an input file only, to avoid blowing up the output size when one
mistake causes a cascade of diagnostics. (This is a matter of designing
the option as e.g. -fdiagnostics-show-caret={no,yes,first} rather than as
-f/fno-, not a matter of needing such a feature implemented in the first
version going on trunk.)
--
Joseph S. Myers
[EMAIL PROTECTED]
Re: [PATCH] caret diagnostics (was: broken FE diagnostics wrt complex expressions)
2008/8/14 Joseph S. Myers <[EMAIL PROTECTED]>: > On Thu, 14 Aug 2008, Manuel López-Ibáñez wrote: > >> It is controlled by -fdiagnostics-show-caret. See the diff for gcc/opts.c. >> >> The configure options are meant to enable/disable all code related to >> caret printing in a similar way as it was done with mapped locations. >> This was requested the first time I sent this patch because it was >> considered too experimental to have it even with >> -fno-diagnostics-show-caret as the default. > > Mapped locations were sitting around on trunk for a long time as a > bitrotten feature; I don't think that's a good example here. When the > code works and passes review it should go on trunk, default remaining the > present diagnostic style according to the GNU Coding Standards, no > configure option. That is nice to know *after* wasting all that time figuring out how configure works [*], ;-) Even if the configure options do not control the compilation of the code, they should at least control the default. I think that, at least for development versions, the default should be on. We do want developers to realise when their new warning/error is pointing to the wrong location. Moreover, some user-friendly distributions may want to enable this by default. Therefore, I would argue to keep the configure options to determine the default value of -fdiagnostics-show-caret. Cheers, Manuel. [*] BTW, thanks to Basile Starynkevitch for writing http://gcc.gnu.org/wiki/Regenerating_GCC_Configuration
Re: [PATCH] caret diagnostics (was: broken FE diagnostics wrt complex expressions)
On Thu, 14 Aug 2008, Manuel López-Ibáñez wrote: > Even if the configure options do not control the compilation of the > code, they should at least control the default. I think that, at least No, whatever the default is there should definitely be no configure option to control it. Configure options controlling such user-visible behavior as this are a very bad idea; users should not find the same options, makefiles etc. acting differently depending on what distribution of the same version of GCC they have. It would be better to remove some configuration options than to add them: make GCC more confident it knows the right way to configure various things for each system and configure them the right way unconditionally. > for development versions, the default should be on. We do want > developers to realise when their new warning/error is pointing to the > wrong location. Moreover, some user-friendly distributions may want to > enable this by default. Therefore, I would argue to keep the configure > options to determine the default value of -fdiagnostics-show-caret. I argue the default should be output (a) compatible with consumers expecting a series of diagnostic lines following the GNU Coding Standards and (b) compact according to existing expectations of GCC output (one line per message instead of three or more, for front ends that have been doing that so far, but probably keeping carets for any front ends that have been using them with their own machinery). Compatibility especially argues for keeping the existing format, but my experience is also that the main use of a diagnostic is to find the right line in an editor and only on a small proportion of occasions (when the warning option could be added to the compilation command manually) is it at all unclear what the problem is, such that more precise details (typically pointing into preprocessed source) would be useful: almost all the time caret diagnostics would make the bulk of diagnostic output into unnecessary noise. But in any case the default should be the default with no configure option, users liking it should find their makefiles work the same everywhere and users not liking it can add the opposite option. -- Joseph S. Myers [EMAIL PROTECTED]
[PATCH][RFT] Optimization pass-pipeline re-organization [1/n]
(cross-posting because of the request for testing below) This is a first step towards getting rid of some passes and re-ordering passes to be more effective and less compile-time consuming. The following measurements have been done on top of some more statistics instrumentation (which I will post as [2/n] later). As a perfect example of benefiting from nearly every pass located anywhere I chose tramp3d-v4.ii as the testcase to look at statistics numbers. The first observation is that the cfg_cleanup pass right after the early inlining pass is doing nothing. Investigation shows why, early inlining already does TODO_cleanup_cfg. Thus I have removed that cfg_cleanup pass. The second observation is that the second update_address_taken pass during early optimization triggers a single(!) time in all of tramp3d at -O3. Thus I have removed that pass (but see below). A third observation is that one simple DSE pass during early optimizations is enough (even though simple DSE would need iterating a few times to no longer find dead statements mainly due to dead struct copies). The first DCE pass after alias computation gets rid of all of the simple dead stores. Thus, I have removed the first simple DSE pass. A fourth observation is that for tramp3d I need to iterate DCE in early optimizations a few times before it finally "converges". As we want to be agressive during early optimizations as far as dead code concerns I have replaced the DCE with control-dependent DCE which does not need iterating (that DCEs 20% more statements during early optimization; for tramp3d it also improves compile-time) A long-standing observation of mine is that the propagation state of addresses before we compute aliasing for the first time is non-optimal - we have still lots of memory accesses through pointers that skew aliasing results mainly due to increased partitioning. Thus, with the goal of doing a similar amount of "cleanup" after the final inlining as we do after early inlining, I moved rename_ssa_copies, complete_unrolli and CCP before build_alias. Noting that build_alias does the equivalent thing to update_address_taken (and we have removed one of them during early optimization) and inlining exposes quite some extra registers (due to propagating parameters passed by reference into their dereference sites), I put the update_address_taken pass back right after the final inlining (it catches about the same number of registers than the one after early inlining). These changes improve tramp3d runtime performance by up to 280%(!) (72s to 25s). This is all for the first round, together with some wrong-code fallout from SRA, some testcase adjustments for moved/changed dumps and an adjustment to the missing alias info warnings from the operand scanner (which actually were what the forwprop-[12].c testcases were matching! duh.) In addition to looking at tramp3d statistics numbers I have run our set of C++ "benchmarks", Polyhedron and SPEC CPU 2000 on Itanium with overall slight improvements in compile-time and run-time. (You can look for yourself at http://gcc.opensuse.org/ for the machine "terbium" and the run right at Aug 14th) Bootstrapped and tested on x86_64-unknown-linux-gnu. I am strongly considering to apply this early next week. So if you have doubts or concerns this is the time to raise them (and back them up with data). Thanks, Richard. 2008-08-14 Richard Guenther <[EMAIL PROTECTED]> * passes.c (init_optimization_passes): Remove cleanup_cfg1, sdse1 and addressables2 passes. Replace dce1 with cddce1. Move call_cdce before build_alias. Move copyrename2, cunrolli and ccp2 beafore build_alias. Re-add addressable2 right after final inlining. * tree-sra.c (generate_element_init_1): Deal with NULL constructor element index. (scalarize_init): If we failed to generate some initializers do not generate zeros for not instantiated members. Instead rely on the copy out. * tree-cfg.c (build_gimple_cfg): Do not dump function here. (pass_build_cfg): But instead via TODO_dump_func. * tree-ssa-operands.c (get_addr_dereference_operands): Warn about missing flow-sensitive alias info only if we have aliases computed. * gcc.dg/fold-alloca-1.c: Scan cfg dump instead of cleanup_cfg1. * gcc.dg/fold-compare-3.c: Likewise. * gcc.dg/tree-ssa/20030709-2.c: Scan cddce2 dump. * gcc.dg/tree-ssa/20030808-1.c: Likewise. * gcc.dg/tree-ssa/20040211-1.c: Likewise. * gcc.dg/tree-ssa/20040305-1.c: Likewise. * gcc.dg/tree-ssa/forwprop-1.c: Adjust pattern. * gcc.dg/tree-ssa/forwprop-2.c: Likewise.. * gcc.dg/tree-ssa/ssa-dce-3.c: Scan cddce1 dump. Index: trunk/gcc/passes.c === *** trunk.orig/gcc/passes.c 2008-08-14 14:11:40.0 +0200 --- trunk/gcc/passes.c 2008-08-14 14:
Re: broken FE diagnostics wrt complex expressions
> There are various issues that would need to be addressed to have > decent caret diagnostics: Agreed. I think having caret diagnostics in place is a good first step, if only because it'll make debugging of column diagnostics easier. After this, we can modify the testsuite machinery to test column accuracy, and have a suite of tests to work from. I'm not sure whether it's best to teach testsuite/lib/*.exp about carets, or just test for :columns:. Anyway, I think it'll be useful to clean up whatever needs cleaning in your patch, test it, and formally submit it. Do you need help testing it? What remains to be done? FWIW, I agree with Joseph that -Wcaret would be better than a configure option. Aldy
Re: [PATCH] caret diagnostics (was: broken FE diagnostics wrt complex expressions)
2008/8/14 Joseph S. Myers <[EMAIL PROTECTED]>: > > But in any case the default should be the default with no configure > option, users liking it should find their makefiles work the same > everywhere and users not liking it can add the opposite option. Then we are not going to get correct locations ever. New users do not read the manual. Neither old users do. New functionality disabled by default will be lost for both. I am fairly sure that a significant percentage of GCC developers (not just users) do not know about -fdiagnostics-show-option. But even more importantly. No GCC developer is going to explicitly enable caret diagnostics while developing GCC. How many nowadays use -fshow-column or -fdiagnostics-show-option to check locations? Moreover, caret diagnostics was mentioned as the way to solve the PRs that Aldy mentioned. If it is disabled by default, how does it solve anything? Why bother? I would really feel that I contributed to make GCC worse if GCC diagnostics are less expressive because we have the excuse of "you could enable the caret". I feel that a lot of PRs that request for better diagnostics would be closed that way. I feel that this thread is going again the same way as the ones before. Someone says: Hey, having caret diagnostics would solve a lot of problems! Everybody says: Yeah, that would be cool! We could do this and that and all kinds of cool things. Then someone says: Oh yes but we need to solve all these boring things that nobody ever really looks and it should be disabled by default. Then one year later someone says: Hey, having caret diagnostics would solve a lot of problems! Cheers, Manuel.
Re: broken FE diagnostics wrt complex expressions
2008/8/14 Aldy Hernandez <[EMAIL PROTECTED]>: >> There are various issues that would need to be addressed to have >> decent caret diagnostics: > > Agreed. I think having caret diagnostics in place is a good first step, > if only because it'll make debugging of column diagnostics easier. > After this, we can modify the testsuite machinery to test column > accuracy, and have a suite of tests to work from. I'm not sure whether > it's best to teach testsuite/lib/*.exp about carets, or just test for > :columns:. > > Anyway, I think it'll be useful to clean up whatever needs cleaning in > your patch, test it, and formally submit it. Do you need help testing > it? What remains to be done? Memory/Compilation time tests. I don't have the framework to test that and it seems the setup is fairly complicated. So that would be really appreciated. > FWIW, I agree with Joseph that -Wcaret would be better than a configure > option. There is -f[no-]diagnostics-show-caret (-W* options are for controlling warnings, we should not use them for anything else). Cheers, Manuel.
Re: [PATCH] caret diagnostics
Manuel López-Ibáñez wrote: 2008/8/14 Joseph S. Myers <[EMAIL PROTECTED]>: But in any case the default should be the default with no configure option, users liking it should find their makefiles work the same everywhere and users not liking it can add the opposite option. Then we are not going to get correct locations ever. New users do not read the manual. Neither old users do. New functionality disabled by default will be lost for both. I am fairly sure that a significant percentage of GCC developers (not just users) do not know about -fdiagnostics-show-option. Users are not beta testers. Forcing inconvenience on users to benefit developers is not justified for a relatively minor issue like this. Changing the default here would have a huge impact, I think probably some developers do not realize the impact that changing messages or output has on people developing large systems for which they expect stable compiler output, e.g. for test cases.
Re: [PATCH] caret diagnostics (was: broken FE diagnostics wrt complex expressions)
On Thu, 14 Aug 2008, Manuel López-Ibáñez wrote: > 2008/8/14 Joseph S. Myers <[EMAIL PROTECTED]>: > > > > But in any case the default should be the default with no configure > > option, users liking it should find their makefiles work the same > > everywhere and users not liking it can add the opposite option. > > Then we are not going to get correct locations ever. New users do not I do not see how your reply relates to the text you quote about not having a configure option, as opposed to the discussion of what the default should be. > read the manual. Neither old users do. New functionality disabled by I certainly did read the manual (the old "Using and Porting") when I first started to use GCC, identified those warning options that seemed good to be and put them in the standard set of warning options I use in my makefiles, and have revised that set from time to time for new versions and absed on experience. > Moreover, caret diagnostics was mentioned as the way to solve the PRs > that Aldy mentioned. If it is disabled by default, how does it solve > anything? Why bother? I would really feel that I contributed to make The solution is producing accurate location ranges, which can be used (a) to print more accurate expressions within the text of diagnostics in the existing style, (b) to print GCS-compliant ranges in text that IDEs can parse to highlight the relevant text in their editors (and we should expect that tools such as GCC and GDB are increasingly going to be used as a back end to other tools rather than just directly on the command line by users), and (c) for caret diagnostics for users liking those on the command line. Caret diagnostics are only one of the styles in which the accurate location information can be used, and implementing an individial style is only a small part of the solution. The PRs are about (a): cases where an expression text is displayed within the existing diagnostic text, badly. Naturally all cases covered by (a) should have tests in the testsuite checking the right text is printed. We should also make the testsuite able to test location ranges for diagnostics that don't include expression text for the relevant range, and then insist in patch review that tests of new front-end diagnostics appropriately assert the range involved, as well as converting existing tests to more precise assertions over time. -- Joseph S. Myers [EMAIL PROTECTED]
Re: [PATCH] caret diagnostics (was: broken FE diagnostics wrt complex expressions)
> Then we are not going to get correct locations ever. New users do not > read the manual. Neither old users do. New functionality disabled by > default will be lost for both. I am fairly sure that a significant > percentage of GCC developers (not just users) do not know about > -fdiagnostics-show-option. Why not get the caret diagnostics in, disabled by default, but with tests that test whatever functionality actually works. Then fix the column information so it's accurate, and finally enable caret diagnostics on by default. And as Joseph said, when the caret diagnostics are enabled, they should be displayed in a format that follow the GNU coding standards for diagnostics. > that Aldy mentioned. If it is disabled by default, how does it solve I envision the caret diagnostics being disabled for only a short while-- while we beat some sense into the column information. There's no point in attacking everything at once. Aldy
Re: [PATCH] caret diagnostics
On Thu, 14 Aug 2008, Robert Dewar wrote: > Manuel López-Ibáñez wrote: > > 2008/8/14 Joseph S. Myers <[EMAIL PROTECTED]>: > > > But in any case the default should be the default with no configure > > > option, users liking it should find their makefiles work the same > > > everywhere and users not liking it can add the opposite option. > > > > Then we are not going to get correct locations ever. New users do not > > read the manual. Neither old users do. New functionality disabled by > > default will be lost for both. I am fairly sure that a significant > > percentage of GCC developers (not just users) do not know about > > -fdiagnostics-show-option. > > Users are not beta testers. Forcing inconvenience on users to benefit > developers is not justified for a relatively minor issue like this. > Changing the default here would have a huge impact, I think probably > some developers do not realize the impact that changing messages or > output has on people developing large systems for which they expect > stable compiler output, e.g. for test cases. We certainly do change the *text* of messages to improve them (this includes putting in more information that can fit within the existing single-line format), and add new messages following the standard formats, but I believe we should leave consumers able to rely on certain aspects of the output that follow the GNU Coding Standards and not start inserting new, non-standard lines in the output that would dominate the standard ones. -- Joseph S. Myers [EMAIL PROTECTED]
Re: [PATCH] caret diagnostics (was: broken FE diagnostics wrt complex expressions)
2008/8/14 Aldy Hernandez <[EMAIL PROTECTED]>: > > I envision the caret diagnostics being disabled for only a short while-- > while we beat some sense into the column information. There's no point > in attacking everything at once. Then, I think we are talking past each other. To be crystal clear, my opinion is: * In the near future, make -fdiagnostics-show-caret the default at least while in experimental mode or at least during stages1 and 2. When making a release -fno-diagnostics-show-caret would be the default. Do this through a configure option that sets the default. * In the far away future, review the defaults and get rid of the configure option. Cheers, Manuel.
Re: [PATCH] caret diagnostics
Joseph S. Myers wrote: We certainly do change the *text* of messages to improve them (this includes putting in more information that can fit within the existing single-line format), and add new messages following the standard formats, but I believe we should leave consumers able to rely on certain aspects of the output that follow the GNU Coding Standards and not start inserting new, non-standard lines in the output that would dominate the standard ones. Right of course improving messages is always reasonable (though in practice at least in the case of Ada, we find that often the major work in updating text of a message is updating all the test base lines :-)) I just think that adding carets everywhere by default is too big a change to be justified.
Re: [PATCH] caret diagnostics
2008/8/14 Robert Dewar <[EMAIL PROTECTED]>: > Manuel López-Ibáñez wrote: >> >> 2008/8/14 Joseph S. Myers <[EMAIL PROTECTED]>: >>> >>> But in any case the default should be the default with no configure >>> option, users liking it should find their makefiles work the same >>> everywhere and users not liking it can add the opposite option. >> >> Then we are not going to get correct locations ever. New users do not >> read the manual. Neither old users do. New functionality disabled by >> default will be lost for both. I am fairly sure that a significant >> percentage of GCC developers (not just users) do not know about >> -fdiagnostics-show-option. > > Users are not beta testers. Forcing inconvenience on users to benefit > developers is not justified for a relatively minor issue like this. > Changing the default here would have a huge impact, I think probably > some developers do not realize the impact that changing messages or > output has on people developing large systems for which they expect > stable compiler output, e.g. for test cases. My proposal is to enable this while not in release mode (like we enable checks and dump statistics and output from the optimizers), so developers notice wrong locations and open a PR and maybe even fix them. Then maybe let users or distributions configure the default as they wish, with our releases defaulting to off. Cheers, Manuel.
Re: [PATCH] caret diagnostics
Manuel López-Ibáñez wrote: My proposal is to enable this while not in release mode (like we enable checks and dump statistics and output from the optimizers), so developers notice wrong locations and open a PR and maybe even fix them. Then maybe let users or distributions configure the default as they wish, with our releases defaulting to off. Won't this have a negative effect on the test infrastructure? It sure would for our Ada testing, where we have hundreds of thousands of diagnostic messages in our test suites. BTW, I am all in favor of caret output, it's not the default in gnat, the default is more like the C default, but -gnatv gives output like: 2.a : integer | >>> missing ";" 4.a := b c; | >>> binary operator expected 5.a := b | >>> missing ";" and -gnatl gives a complete listing Compiling: k.adb 1. procedure k is 2.a : integer | >>> missing ";" 3. begin 4.a := b c; | >>> binary operator expected 5.a := b | >>> missing ";" 6.c; 7. end; This last listing is interesting, it shows how GNAT pays attention to code layout in diagnostics, giving quite different messages for two cases of the same token sequence. FWIW we find most users are aware of options like this. We make a special attempt to encourage people to at least read through the list of switches, even if they read no other documentation.
Re: [PATCH] caret diagnostics (was: broken FE diagnostics wrt complex expressions)
2008/8/14 Joseph S. Myers <[EMAIL PROTECTED]>: > > The solution is producing accurate location ranges, which can be used (a) > to print more accurate expressions within the text of diagnostics in the > existing style, (b) to print GCS-compliant ranges in text that IDEs can > parse to highlight the relevant text in their editors (and we should > expect that tools such as GCC and GDB are increasingly going to be used as > a back end to other tools rather than just directly on the command line by > users), and (c) for caret diagnostics for users liking those on the > command line. Caret diagnostics are only one of the styles in which the > accurate location information can be used, and implementing an individial > style is only a small part of the solution. The PRs are about (a): cases > where an expression text is displayed within the existing diagnostic text, > badly. > Then, we are talking about different things and having caret diagnostics is not the solution to anything of the above or the PRs mentioned in the original thread but a consequence of the solution. Thanks for clarifying this. Sorry for hijacking the thread. Cheers, Manuel.
Re: [PATCH] caret diagnostics
2008/8/14 Robert Dewar <[EMAIL PROTECTED]>: > Manuel López-Ibáñez wrote: > >> My proposal is to enable this while not in release mode (like we >> enable checks and dump statistics and output from the optimizers), so >> developers notice wrong locations and open a PR and maybe even fix >> them. Then maybe let users or distributions configure the default as >> they wish, with our releases defaulting to off. > > Won't this have a negative effect on the test infrastructure? It > sure would for our Ada testing, where we have hundreds of thousands > of diagnostic messages in our test suites. Even when enabled with -fdiagnostics-show-caret and configured with --enable-languages=all,ada,obj-c++, all acats and gnat tests pass. Of course, we should disable it with -fno-diagnostics-show-caret as we currently do for -fshow-colum in lib/gcc.exp. You'll notice that is what my patch does if you read it. Cheers, Manuel.
Re: [PATCH] caret diagnostics
On Thu, 14 Aug 2008, Robert Dewar wrote: > BTW, I am all in favor of caret output, it's not the default in > gnat, the default is more like the C default, but -gnatv gives > output like: And I'd hope we could keep things that way for both C and Ada, but make similar modes to the Ada ones available for C (with Ada treating the common compiler options as synonyms for the the existing Ada options). In fact I'd hope we could get Ada using the shared diagnostic infrastructure, i18n support etc., but realise that's a lot of work (a lot needed doing just to get *almost all* C diagnostics in suitable shape for i18n) and might have issues with using the same Ada sources with different compiler back end versions. -- Joseph S. Myers [EMAIL PROTECTED]
Re: [PATCH] caret diagnostics
> "Joseph" == Joseph S Myers <[EMAIL PROTECTED]> writes: Joseph> (b) to print GCS-compliant ranges in text that IDEs can parse Joseph> to highlight the relevant text in their editors Joseph> Caret diagnostics are only one of the styles in which the Joseph> accurate location information can be used, and implementing an Joseph> individial style is only a small part of the solution. Yes, I agree, we need multiple things: accurate locations from the front ends (ideally macro-expansion-aware), start- and end-locations, better diagnostic output of various kinds, perhaps smarter location handling in the optimizations, and of course finally column output in dwarf. I hope that covers the 80% of stuff we all seem to agree on. I'm sympathetic to the idea that switching to caret output by default will break things. However, I don't think that GCS-style ranges are necessarily any more reality-proof, because I am skeptical that most tool developers read this document when deciding how to parse GCC's output. (I'm guessing that plain column output is ok, since libcpp already does that.) I'd like to see carets on by default as part of a major release -- say GCC 5.0. (First mention!!) Manuel's idea that we should enable column- or caret-output in the development (but not release) GCC is worthy of consideration. We certainly aren't seeing much progress on this front as-is, maybe this change would inspire GCC developers a bit. It will also help root out the non-GCS-complaint tools ;) Tom
Re: [PATCH] caret diagnostics
On Thu, Aug 14, 2008 at 6:52 PM, Tom Tromey <[EMAIL PROTECTED]> wrote: > I'd like to see carets on by default as part of a major release -- say > GCC 5.0. (First mention!!) Whoa, you wish :-) Honors go to Marc Espie here: http://gcc.gnu.org/ml/gcc/2000-12/msg00636.html But there've been several other mails about gcc 5.0 since then, too. Maybe LTO is user-visible and lord-knows-what-breaking enough for a major version number bump 5.0. Caret diagnostics could piggyback on that :-) Gr. Steven
Re: [PATCH] caret diagnostics
* Tom Tromey wrote on Thu, Aug 14, 2008 at 06:52:24PM CEST: > I'm sympathetic to the idea that switching to caret output by default > will break things. However, I don't think that GCS-style ranges are > necessarily any more reality-proof, because I am skeptical that most > tool developers read this document when deciding how to parse GCC's > output. The GCS document helps not only to let tool developers know how output of programs they parse looks like; more importantly, it helps ensure that output from different programs is consistent. If the GCS-style is not powerful enough to meet GCC's needs, then please let's not only improve GCC, but the GCS document also, so that other programs improve compatibly. Cheers, Ralf
Re: [PATCH] caret diagnostics
On Aug 14, 2008, at 8:47 AM, Joseph S. Myers wrote: On Thu, 14 Aug 2008, Robert Dewar wrote: BTW, I am all in favor of caret output, it's not the default in gnat, the default is more like the C default, but -gnatv gives output like: And I'd hope we could keep things that way for both C and Ada, but make Please don't forget C++. -Chris
Re: [PATCH] caret diagnostics (was: broken FE diagnostics wrt complex expressions)
> * In the near future, make -fdiagnostics-show-caret the default at > least while in experimental mode or at least during stages1 and 2. > When making a release -fno-diagnostics-show-caret would be the > default. Do this through a configure option that sets the default. > > * In the far away future, review the defaults and get rid of the > configure option. To be honest, I don't mind either way. Either your option or Joseph's is fine. I am however interested in getting the caret diagnostics in, and then I can work on location information that will benefit everyone. Aldy
Re: [PATCH] caret diagnostics
On Thu, 14 Aug 2008, Tom Tromey wrote: > Manuel's idea that we should enable column- or caret-output in the > development (but not release) GCC is worthy of consideration. We > certainly aren't seeing much progress on this front as-is, maybe this > change would inspire GCC developers a bit. It will also help root out > the non-GCS-complaint tools ;) I think making this different in development would be a mistake. It's not like --enable-checking; it's something user-visible. -- Joseph S. Myers [EMAIL PROTECTED]
Re: [PATCH] caret diagnostics
On Thu, 14 Aug 2008, Ralf Wildenhues wrote: > * Tom Tromey wrote on Thu, Aug 14, 2008 at 06:52:24PM CEST: > > I'm sympathetic to the idea that switching to caret output by default > > will break things. However, I don't think that GCS-style ranges are > > necessarily any more reality-proof, because I am skeptical that most > > tool developers read this document when deciding how to parse GCC's > > output. > > The GCS document helps not only to let tool developers know how output > of programs they parse looks like; more importantly, it helps ensure > that output from different programs is consistent. And thereby allows a post-processor to add caret diagnostics to the output of any GCS-conforming program, or an IDE or editor to show the locations of errors from such a program, not just GCC, for example. -- Joseph S. Myers [EMAIL PROTECTED]
Re: [PATCH] caret diagnostics
On Thu, 14 Aug 2008, Tom Tromey wrote: > Yes, I agree, we need multiple things: accurate locations from the > front ends (ideally macro-expansion-aware), start- and end-locations, > better diagnostic output of various kinds, perhaps smarter location > handling in the optimizations, and of course finally column output in > dwarf. I hope that covers the 80% of stuff we all seem to agree on. Regarding the list of improvements, I think most of the benefits from caret diagnostics can be gained simply by printing accurate texts of expressions (or declarations etc.) within the existing one-line diagnostic messages, without needing major and incompatible stylistic changes. -- Joseph S. Myers [EMAIL PROTECTED]
Re: [PATCH] caret diagnostics
> "Ralf" == Ralf Wildenhues <[EMAIL PROTECTED]> writes: Ralf> If the GCS-style is not powerful enough to meet GCC's needs, then please Ralf> let's not only improve GCC, but the GCS document also, so that other Ralf> programs improve compatibly. Yeah, good idea. FWIW -- the gcc-output-parsing tools I care most about are actually usually parsing the output of 'make', which is already full of random undigestible text that must be ignored. Caret diagnostics are extremely unlike to break these tools. Tom
gcc@gcc.gnu.org
Dear all,
In order to fix PR 179, I need help either understanding why exactly
the warning is triggered or obtaining a small self-contained testcase
to reproduce it.
Thanks in advance,
Manuel.
The attached patch triggers the warnings:
/home/manuel/src/trunk/gcc/builtins.c: In function 'fold_builtin_strchr':
/home/manuel/src/trunk/gcc/builtins.c:11095: error: 'c' is used
uninitialized in this function
/home/manuel/src/trunk/gcc/builtins.c: In function 'fold_builtin_memchr':
/home/manuel/src/trunk/gcc/builtins.c:8963: error: 'c' is used
uninitialized in this function
Uncommenting the following avoids the warning:
+ /*
+ if (is_call_clobbered (var))
+{
+ var_ann_t va = var_ann (var);
+ unsigned int escape_mask = va->escape_mask;
+ if (escape_mask & ESCAPE_TO_ASM)
+ return false;
+ if (escape_mask & ESCAPE_IS_GLOBAL)
+ return false;
+ if (escape_mask & ESCAPE_IS_PARM)
+ return false;
+}
+ */
The alias dump is:
fold_builtin_strchr (union tree_node * s1D.68612, union tree_node *
s2D.68613, union tree_node * typeD.68614)
{
union tree_node * temD.68620;
const charD.1 * rD.68619;
charD.1 cD.68618;
const charD.1 * p1D.68617;
union tree_node * D.68650;
union tree_node * D.68649;
long intD.2 D.68648;
long intD.2 p1.2917D.68647;
long intD.2 r.2916D.68646;
union tree_node * D.68644;
intD.0 D.68641;
charD.1 c.2915D.68640;
intD.0 D.68637;
short unsigned intD.8 D.68631;
union tree_node * D.68630;
unsigned charD.10 D.68629;
unsigned charD.10 D.68627;
# BLOCK 2 freq:1
# PRED: ENTRY [100.0%] (fallthru,exec)
[/home/manuel/src/trunk/gcc/builtins.c : 11095] # VUSE
{ cD.68618 }
D.68627_3 = validate_argD.45737 (s1D.68612_2(D), 10);
[/home/manuel/src/trunk/gcc/builtins.c : 11095] if (D.68627_3 == 0)
goto ;
else
goto ;
# SUCC: 10 [95.7%] (true,exec) 3 [4.3%] (false,exec)
# BLOCK 3 freq:434
# PRED: 2 [4.3%] (false,exec)
[/home/manuel/src/trunk/gcc/builtins.c : 11095] # VUSE
{ cD.68618 }
D.68629_5 = validate_argD.45737 (s2D.68613_4(D), 8);
[/home/manuel/src/trunk/gcc/builtins.c : 11095] if (D.68629_5 == 0)
goto ;
else
goto ;
# SUCC: 10 [90.0%] (true,exec) 4 [10.0%] (false,exec)
# BLOCK 4 freq:43
# PRED: 3 [10.0%] (false,exec)
[/home/manuel/src/trunk/gcc/builtins.c : 11102] # VUSE
{ cD.68618 SMT.3811D.75594 }
D.68631_6 = s2D.68613_4(D)->baseD.20795.codeD.19700;
[/home/manuel/src/trunk/gcc/builtins.c : 11102] if (D.68631_6 != 23)
goto ;
else
goto ;
# SUCC: 10 [98.3%] (true,exec) 5 [1.7%] (false,exec)
# BLOCK 5 freq:1
# PRED: 4 [1.7%] (false,exec)
[/home/manuel/src/trunk/gcc/builtins.c : 11105] # cD.68618_37 = VDEF
# SMT.3811D.75594_38 = VDEF
# SMT.3812D.75595_39 = VDEF { cD.68618
SMT.3811D.75594 SMT.3812D.75595 }
p1D.68617_8 = c_getstrD.45477 (s1D.68612_2(D));
[/home/manuel/src/trunk/gcc/builtins.c : 11106] if (p1D.68617_8 != 0B)
goto ;
else
goto ;
# SUCC: 6 [20.5%] (true,exec) 10 [79.5%] (false,exec)
# BLOCK 6
# PRED: 5 [20.5%] (true,exec)
[/home/manuel/src/trunk/gcc/builtins.c : 2] # cD.68618_40 = VDEF
# SMT.3811D.75594_41 = VDEF
# SMT.3812D.75595_42 = VDEF { cD.68618
SMT.3811D.75594 SMT.3812D.75595 }
D.68637_10 = target_char_castD.45483 (s2D.68613_4(D), &cD.68618);
[/home/manuel/src/trunk/gcc/builtins.c : 2] if (D.68637_10 != 0)
goto ;
else
goto ;
# SUCC: 10 [39.0%] (true,exec) 7 [61.0%] (false,exec)
# BLOCK 7
# PRED: 6 [61.0%] (false,exec)
[/home/manuel/src/trunk/gcc/builtins.c : 5] # VUSE
{ cD.68618 }
c.2915D.68640_12 = cD.68618;
[/home/manuel/src/trunk/gcc/builtins.c : 5] D.68641_13 =
(intD.0) c.2915D.68640_12;
[/home/manuel/src/trunk/gcc/builtins.c : 5] # VUSE
{ cD.68618 }
rD.68619_14 = strchrD.689 (p1D.68617_8, D.68641_13);
[/home/manuel/src/trunk/gcc/builtins.c : 7] if (rD.68619_14 == 0B)
goto ;
else
goto ;
# SUCC: 8 [10.1%] (true,exec) 9 [89.9%] (false,exec)
# BLOCK 9
# PRED: 7 [89.9%] (false,exec)
[/home/manuel/src/trunk/gcc/builtins.c : 11121] r.2916D.68646_20 =
(long intD.2) rD.68619_14;
[/home/manuel/src/trunk/gcc/builtins.c : 11121] p1.2917D.68647_21 =
(long intD.2) p1D.68617_8;
[/home/manuel/src/trunk/gcc/builtins.c : 11121] D.68648_22 =
r.2916D.68646_20 - p1.2917D.68647_21;
[/home/manuel/src/trunk/gcc/builtins.c : 11121] # cD.68618_46 = VDEF
# SMT.3811D.75594_47 = VDEF
# SMT.3812D.75595_48 = VDEF { cD.68618
SMT.3811D.75594 SMT.3812D.75595 }
D.68649_23 = size_int_kindD.21427 (D.68648_22, 0);
[/home/manuel/src/trunk/gcc/builtins.c : 11121] # VUSE { cD.68618 SMT.3811D.75594 }
D.68650_26 = s1D.68612_2(D)->commonD.20796.typeD.19731;
[/home/manuel/src/trunk/gcc/builtins.c : 11121] # cD.68618_49 = VDEF
# SMT.3811D.75594_50 = VDEF
# SMT.3812D.75595_51 = VDEF { cD.68618
SMT.3811D.75594 SMT.3812D.75595 }
temD.68620_27 = fold_build2_statD.21716 (67, D.68650_26,
s1D.68612_2(D), D.68649_23);
Re: [PATCH] caret diagnostics
* Tom Tromey wrote on Thu, Aug 14, 2008 at 07:39:57PM CEST: > > FWIW -- the gcc-output-parsing tools I care most about are actually > usually parsing the output of 'make', which is already full of random > undigestible text that must be ignored. Caret diagnostics are > extremely unlike to break these tools. But there are editors which are able to parse caret diagnostics from compilers. It's quite helpful for them if caret diagnostic output is consistent among compiler/compiler-like tools. I care about editors :) Cheers, Ralf
Re: [PATCH] caret diagnostics
Manuel López-Ibáñez wrote: My proposal is to enable this while not in release mode (like we enable checks and dump statistics and output from the optimizers), so developers notice wrong locations and open a PR and maybe even fix them. Manuel -- I think it's great that you're working on caret diagnostics. However, I agree 100% with Joseph and Robert. Users, including other GCC developers, are not beta testers. We can all work together, but the approach of "break the compiler and hope that other developers will fix it" is not a good plan. As Joseph says, this needs to be configured off by default (for compatibility with the way GCC has worked since forever, and for compatibility with the GNU standards), and there needs to be an option to turn it on. You will need to recruit others who are willing to invest in helping to improve the new mode's quality. Thanks, -- Mark Mitchell CodeSourcery [EMAIL PROTECTED] (650) 331-3385 x713
Re: [PATCH] caret diagnostics
Joseph S. Myers wrote: On Thu, 14 Aug 2008, Tom Tromey wrote: Yes, I agree, we need multiple things: accurate locations from the front ends (ideally macro-expansion-aware), start- and end-locations, better diagnostic output of various kinds, perhaps smarter location handling in the optimizations, and of course finally column output in dwarf. I hope that covers the 80% of stuff we all seem to agree on. Regarding the list of improvements, I think most of the benefits from caret diagnostics can be gained simply by printing accurate texts of expressions (or declarations etc.) within the existing one-line diagnostic messages, without needing major and incompatible stylistic changes. Indeed. The important thing is not the caret per se; it's having accurate line/column information for diagnostics and using the text the user wrote, rather than trying to reconstruct expressions. The caret output format may be desirable to some users as well, but most of the work here will be in getting the locations correct and in showing the user's original text. -- Mark Mitchell CodeSourcery [EMAIL PROTECTED] (650) 331-3385 x713
Re: [PATCH] caret diagnostics
Ralf Wildenhues wrote: * Tom Tromey wrote on Thu, Aug 14, 2008 at 07:39:57PM CEST: FWIW -- the gcc-output-parsing tools I care most about are actually usually parsing the output of 'make', which is already full of random undigestible text that must be ignored. Caret diagnostics are extremely unlike to break these tools. But there are editors which are able to parse caret diagnostics from compilers. It's quite helpful for them if caret diagnostic output is consistent among compiler/compiler-like tools. I care about editors :) Makes more sense for editors to parse column numbers, which they already do just fine, seems pointless for them to mess with carets. Cheers, Ralf
Re: [PATCH] caret diagnostics
2008/8/14 Mark Mitchell <[EMAIL PROTECTED]>: > Manuel López-Ibáñez wrote: > >> My proposal is to enable this while not in release mode (like we >> enable checks and dump statistics and output from the optimizers), so >> developers notice wrong locations and open a PR and maybe even fix >> them. > > Manuel -- > > We can all work together, but the approach of "break the compiler and hope > that other developers will fix it" is not a good plan. As Joseph says, this > needs to be configured off by default (for compatibility with the way GCC > has worked since forever, and for compatibility with the GNU standards), and > there needs to be an option to turn it on. You will need to recruit others > who are willing to invest in helping to improve the new mode's quality. The locations are as wrong now as they will be after enabling caret diagnostics. The only difference is that no one pays attention right now because that would require to enable -fshow-column and count column numbers. So I am certainly not "breaking the compiler and hoping that other developers will fix it". This is *exactly* like printing statistics at the end of the compilation. It may hint you that something is broken somewhere but you can freely ignore it as a visual hassle of the experimental compiler. BTW, all those time/memory statistics didn't break anything when they were added? And they are given even if no error/warning is printed. Cheers, Manuel.
Re: [PATCH] caret diagnostics
Manuel López-Ibáñez wrote: The locations are as wrong now as they will be after enabling caret diagnostics. The only difference is that no one pays attention right now because that would require to enable -fshow-column and count column numbers. So I am certainly not "breaking the compiler and hoping that other developers will fix it". OK, instead of "break", I will say that you are hoping to change the user-visible behavior in a way that makes a current limitation obvious, in the hope that will prod people to fix it. I don't think that's appropriate. Having cc1 print a statistic (when not passed "-quiet") saying "I got error messages wrong 100 times in this translation unit" would be fine (if it were possible to compute) since it wouldn't get in anyone's way. Your change will inconvenience people. Please attack the problem directly, by working with others to improve the location information in the compiler. My experience has been that if you get the ball rolling, other people will get behind and help, when they see that the problem is tractable. Thanks, -- Mark Mitchell CodeSourcery [EMAIL PROTECTED] (650) 331-3385 x713
Bootstrap broken on x86_64-linux
Failure: ../../../libgfortran/intrinsics/cshift0.c: In function 'cshift0': ../../../libgfortran/intrinsics/cshift0.c:124: warning: passing argument 1 of 'cshift0_i16' from incompatible pointer type ../../../libgfortran/intrinsics/cshift0.c:236: error: 'GFC_INTGER_16' undeclared (first use in this function) ../../../libgfortran/intrinsics/cshift0.c:236: error: (Each undeclared identifier is reported only once ../../../libgfortran/intrinsics/cshift0.c:236: error: for each function it appears in.) make[3]: *** [cshift0.lo] Error 1 make[3]: *** Waiting for unfinished jobs Caused by: Changed by: tkoenig Changed at: Thu 14 Aug 2008 14:38:46 Revision: 139111 Changed files: libgfortran/generated/cshift0_r4.c libgfortran/ChangeLog libgfortran/generated/cshift0_c16.c libgfortran/generated/cshift0_r8.c libgfortran/generated/cshift0_i16.c libgfortran/libgfortran.h libgfortran/m4/cshift0.m4 libgfortran/generated/cshift0_r10.c gcc/testsuite/ChangeLog libgfortran/generated/cshift0_c4.c libgfortran/intrinsics/cshift0.c libgfortran/generated/cshift0_r16.c libgfortran/generated/cshift0_i1.c libgfortran/Makefile.am libgfortran/generated/cshift0_c8.c libgfortran/generated/cshift0_i2.c libgfortran/generated/cshift0_i4.c libgfortran/generated/cshift0_i8.c gcc/testsuite/gfortran.dg/cshift_nan_1.f90 libgfortran/generated/cshift0_c10.c libgfortran/Makefile.in gcc/testsuite/gfortran.dg/char_cshift_3.f90 Comments: 2008-08-14 Thomas Koenig <[EMAIL PROTECTED]> PR libfortran/36886 * Makefile.am: Added $(i_cshift0_c). Added $(i_cshift0_c) to gfor_built_specific_src. Add rule to build from cshift0.m4. * Makefile.in: Regenerated. * libgfortran.h: Addedd prototypes for cshift0_i1, cshift0_i2, cshift0_i4, cshift0_i8, cshift0_i16, cshift0_r4, cshift0_r8, cshift0_r10, cshift0_r16, cshift0_c4, cshift0_c8, cshift0_c10, cshift0_c16. Define Macros GFC_UNALIGNED_C4 and GFC_UNALIGNED_C8. * intrinsics/cshift0.c: Remove helper functions for the innter shift loop. (cshift0): Call specific functions depending on type of array argument. Only call specific functions for correct alignment for other types. * m4/cshift0.m4: New file. * generated/cshift0_i1.c: New file. * generated/cshift0_i2.c: New file. * generated/cshift0_i4.c: New file. * generated/cshift0_i8:.c New file. * generated/cshift0_i16.c: New file. * generated/cshift0_r4.c: New file. * generated/cshift0_r8.c: New file. * generated/cshift0_r10.c: New file. * generated/cshift0_r16.c: New file. * generated/cshift0_c4.c: New file. * generated/cshift0_c8.c: New file. * generated/cshift0_c10.c: New file. * generated/cshift0_c16.c: New file. 2008-08-14 Thomas Koenig <[EMAIL PROTECTED]> PR libfortran/36886 * gfortran.dg/cshift_char_3.f90: New test case. * gfortran.dg/cshift_nan_1.f90: New test case.
gcc@gcc.gnu.org
1. You can't assume VUSE's are must-aliases. The fact that there is a
vuse for something does not imply it is must-used, it implies it is
may-used.
We do not differentiate may-use from must-use in our alias system. You
can do some trivial must-use analysis if you like (by computing
cardinality of points-to set as either single or multiple and
propagating/meeting it in the right place).
Must-use is actually quite rare.
2. " if (!gimple_references_memory_p (def))
+ return;
+"
Is nonsensical the SSA_NAME_DEF_STMT of a vuse must contain a vdef,
and thus must access memory.
On Thu, Aug 14, 2008 at 2:16 PM, Manuel López-Ibáñez
<[EMAIL PROTECTED]> wrote:
> Dear all,
>
> In order to fix PR 179, I need help either understanding why exactly
> the warning is triggered or obtaining a small self-contained testcase
> to reproduce it.
>
> Thanks in advance,
>
> Manuel.
>
> The attached patch triggers the warnings:
>
> /home/manuel/src/trunk/gcc/builtins.c: In function 'fold_builtin_strchr':
> /home/manuel/src/trunk/gcc/builtins.c:11095: error: 'c' is used
> uninitialized in this function
> /home/manuel/src/trunk/gcc/builtins.c: In function 'fold_builtin_memchr':
> /home/manuel/src/trunk/gcc/builtins.c:8963: error: 'c' is used
> uninitialized in this function
>
> Uncommenting the following avoids the warning:
>
> + /*
> + if (is_call_clobbered (var))
> +{
> + var_ann_t va = var_ann (var);
> + unsigned int escape_mask = va->escape_mask;
> + if (escape_mask & ESCAPE_TO_ASM)
> + return false;
> + if (escape_mask & ESCAPE_IS_GLOBAL)
> + return false;
> + if (escape_mask & ESCAPE_IS_PARM)
> + return false;
> +}
> + */
>
> The alias dump is:
>
> fold_builtin_strchr (union tree_node * s1D.68612, union tree_node *
> s2D.68613, union tree_node * typeD.68614)
> {
> union tree_node * temD.68620;
> const charD.1 * rD.68619;
> charD.1 cD.68618;
> const charD.1 * p1D.68617;
> union tree_node * D.68650;
> union tree_node * D.68649;
> long intD.2 D.68648;
> long intD.2 p1.2917D.68647;
> long intD.2 r.2916D.68646;
> union tree_node * D.68644;
> intD.0 D.68641;
> charD.1 c.2915D.68640;
> intD.0 D.68637;
> short unsigned intD.8 D.68631;
> union tree_node * D.68630;
> unsigned charD.10 D.68629;
> unsigned charD.10 D.68627;
>
> # BLOCK 2 freq:1
> # PRED: ENTRY [100.0%] (fallthru,exec)
> [/home/manuel/src/trunk/gcc/builtins.c : 11095] # VUSE
> { cD.68618 }
> D.68627_3 = validate_argD.45737 (s1D.68612_2(D), 10);
> [/home/manuel/src/trunk/gcc/builtins.c : 11095] if (D.68627_3 == 0)
>goto ;
> else
>goto ;
> # SUCC: 10 [95.7%] (true,exec) 3 [4.3%] (false,exec)
>
> # BLOCK 3 freq:434
> # PRED: 2 [4.3%] (false,exec)
> [/home/manuel/src/trunk/gcc/builtins.c : 11095] # VUSE
> { cD.68618 }
> D.68629_5 = validate_argD.45737 (s2D.68613_4(D), 8);
> [/home/manuel/src/trunk/gcc/builtins.c : 11095] if (D.68629_5 == 0)
>goto ;
> else
>goto ;
> # SUCC: 10 [90.0%] (true,exec) 4 [10.0%] (false,exec)
>
> # BLOCK 4 freq:43
> # PRED: 3 [10.0%] (false,exec)
> [/home/manuel/src/trunk/gcc/builtins.c : 11102] # VUSE
> { cD.68618 SMT.3811D.75594 }
> D.68631_6 = s2D.68613_4(D)->baseD.20795.codeD.19700;
> [/home/manuel/src/trunk/gcc/builtins.c : 11102] if (D.68631_6 != 23)
>goto ;
> else
>goto ;
> # SUCC: 10 [98.3%] (true,exec) 5 [1.7%] (false,exec)
>
> # BLOCK 5 freq:1
> # PRED: 4 [1.7%] (false,exec)
> [/home/manuel/src/trunk/gcc/builtins.c : 11105] # cD.68618_37 = VDEF
>
> # SMT.3811D.75594_38 = VDEF
> # SMT.3812D.75595_39 = VDEF { cD.68618
> SMT.3811D.75594 SMT.3812D.75595 }
> p1D.68617_8 = c_getstrD.45477 (s1D.68612_2(D));
> [/home/manuel/src/trunk/gcc/builtins.c : 11106] if (p1D.68617_8 != 0B)
>goto ;
> else
>goto ;
> # SUCC: 6 [20.5%] (true,exec) 10 [79.5%] (false,exec)
>
> # BLOCK 6
> # PRED: 5 [20.5%] (true,exec)
> [/home/manuel/src/trunk/gcc/builtins.c : 2] # cD.68618_40 = VDEF
>
> # SMT.3811D.75594_41 = VDEF
> # SMT.3812D.75595_42 = VDEF { cD.68618
> SMT.3811D.75594 SMT.3812D.75595 }
> D.68637_10 = target_char_castD.45483 (s2D.68613_4(D), &cD.68618);
> [/home/manuel/src/trunk/gcc/builtins.c : 2] if (D.68637_10 != 0)
>goto ;
> else
>goto ;
> # SUCC: 10 [39.0%] (true,exec) 7 [61.0%] (false,exec)
>
> # BLOCK 7
> # PRED: 6 [61.0%] (false,exec)
> [/home/manuel/src/trunk/gcc/builtins.c : 5] # VUSE
> { cD.68618 }
> c.2915D.68640_12 = cD.68618;
> [/home/manuel/src/trunk/gcc/builtins.c : 5] D.68641_13 =
> (intD.0) c.2915D.68640_12;
> [/home/manuel/src/trunk/gcc/builtins.c : 5] # VUSE
> { cD.68618 }
> rD.68619_14 = strchrD.689 (p1D.68617_8, D.68641_13);
> [/home/manuel/src/trunk/gcc/builtins.c : 7] if (rD.68619_14 == 0B)
>goto ;
> else
>goto ;
> # SUCC: 8 [10.1%] (true,exec) 9 [89.9%] (false,exec)
>
> # BLOCK 9
> # PRED: 7 [89.9%] (false,exec)
> [/home/manuel/src/trunk/gcc/builtins.c : 11121] r.2916D.68646_20 =
> (long intD.2) rD.68
Re: Bootstrap broken on x86_64-linux
On Thu, Aug 14, 2008 at 1:32 PM, Daniel Berlin <[EMAIL PROTECTED]> wrote: > Failure: > > ../../../libgfortran/intrinsics/cshift0.c: In function 'cshift0': > ../../../libgfortran/intrinsics/cshift0.c:124: warning: passing > argument 1 of 'cshift0_i16' from incompatible pointer type > ../../../libgfortran/intrinsics/cshift0.c:236: error: 'GFC_INTGER_16' > undeclared (first use in this function) > ../../../libgfortran/intrinsics/cshift0.c:236: error: (Each undeclared > identifier is reported only once > ../../../libgfortran/intrinsics/cshift0.c:236: error: for each > function it appears in.) > make[3]: *** [cshift0.lo] Error 1 > make[3]: *** Waiting for unfinished jobs > It should be fixed now. H.J.
gcc-4.3-20080814 is now available
Snapshot gcc-4.3-20080814 is now available on ftp://gcc.gnu.org/pub/gcc/snapshots/4.3-20080814/ and on various mirrors, see http://gcc.gnu.org/mirrors.html for details. This snapshot has been generated from the GCC 4.3 SVN branch with the following options: svn://gcc.gnu.org/svn/gcc/branches/gcc-4_3-branch revision 139117 You'll find: gcc-4.3-20080814.tar.bz2 Complete GCC (includes all of below) gcc-core-4.3-20080814.tar.bz2 C front end and core compiler gcc-ada-4.3-20080814.tar.bz2 Ada front end and runtime gcc-fortran-4.3-20080814.tar.bz2 Fortran front end and runtime gcc-g++-4.3-20080814.tar.bz2 C++ front end and runtime gcc-java-4.3-20080814.tar.bz2 Java front end and runtime gcc-objc-4.3-20080814.tar.bz2 Objective-C front end and runtime gcc-testsuite-4.3-20080814.tar.bz2The GCC testsuite Diffs from 4.3-20080807 are available in the diffs/ subdirectory. When a particular snapshot is ready for public consumption the LATEST-4.3 link is updated and a message is sent to the gcc list. Please do not use a snapshot before it has been announced that way.
Re: [PATCH][RFT] Optimization pass-pipeline re-organization [1/n]
Richard Guenther wrote: These changes improve tramp3d runtime performance by up to 280%(!) (72s to 25s). Great! Can you check the impact on PR33604? Paolo
