| Message ID | E09269D8-43D5-4D3B-88E2-AE4EC75FDA1A@adacore.com |
|---|---|
| State | New |
| Headers | show |
On Tue, 6 Mar 2012, Tristan Gingold wrote: > The patch is simple: the C front-end will now calls c_build_pointer_type > (instead of build_pointer_type), which in turn calls > build_pointer_type_for_mode using the right mode. There seem to be quite a lot of build_pointer_type calls in the C front end (and in c-common.c) that you haven't changed. Could you explain the rule for when a call should or should not be changed, and how it applies to all these calls?
On Mar 6, 2012, at 6:34 PM, Joseph S. Myers wrote: > On Tue, 6 Mar 2012, Tristan Gingold wrote: > >> The patch is simple: the C front-end will now calls c_build_pointer_type >> (instead of build_pointer_type), which in turn calls >> build_pointer_type_for_mode using the right mode. > > There seem to be quite a lot of build_pointer_type calls in the C front > end (and in c-common.c) that you haven't changed. Could you explain the > rule for when a call should or should not be changed, and how it applies > to all these calls? The global approach is to have the same effect as a default __attribute__((mode(SI/DImode))) on pointers declared by users so that layouts match. That's why only grokdeclarator is changed. There might be bugs with this approach (e.g. it looks like c-common.c:handle_noreturn_attribute doesn't preserve the mode of the pointer to function), but my understanding is that they also correspond to bugs of __attribute__((mode ())) applied to pointer. The later one isn't well tested and one advantage of the VMS port is that it will test it much more (as there are many pragma pointer_size in VMS headers). I haven't tried to exactly follow the DEC-C rules, because: - they aren't formally written - the current behavior of the front and middle end is already correct I agree that it is possible to concoct tests (e.g. using sizeof) that will return different results between gcc and DEC-C, but my purpose is to be reasonable compatible, not fully compatible. There are some features (such as supporting VAX float) that I don't plan to implement :-) Tristan.
On Wed, 7 Mar 2012, Tristan Gingold wrote: > On Mar 6, 2012, at 6:34 PM, Joseph S. Myers wrote: > > > On Tue, 6 Mar 2012, Tristan Gingold wrote: > > > >> The patch is simple: the C front-end will now calls c_build_pointer_type > >> (instead of build_pointer_type), which in turn calls > >> build_pointer_type_for_mode using the right mode. > > > > There seem to be quite a lot of build_pointer_type calls in the C front > > end (and in c-common.c) that you haven't changed. Could you explain the > > rule for when a call should or should not be changed, and how it applies > > to all these calls? > > The global approach is to have the same effect as a default > __attribute__((mode(SI/DImode))) on pointers declared by users so that > layouts match. That's why only grokdeclarator is changed. > > There might be bugs with this approach (e.g. it looks like > c-common.c:handle_noreturn_attribute doesn't preserve the mode of the > pointer to function), but my understanding is that they also correspond > to bugs of __attribute__((mode ())) applied to pointer. The later one > isn't well tested and one advantage of the VMS port is that it will test > it much more (as there are many pragma pointer_size in VMS headers). So those places would need to change to use build_pointer_type_for_mode as is done for composite types in c-typeck.c:composite_type, for example? I think the patch at least needs a (VMS-specific?) testcase that tests the new pragma (presuming the testsuite can be run for VMS targets) even if some cases can't be tested until they are fixed.
On Mar 7, 2012, at 7:42 PM, Joseph S. Myers wrote: > On Wed, 7 Mar 2012, Tristan Gingold wrote: > >> On Mar 6, 2012, at 6:34 PM, Joseph S. Myers wrote: >> >>> On Tue, 6 Mar 2012, Tristan Gingold wrote: >>> >>>> The patch is simple: the C front-end will now calls c_build_pointer_type >>>> (instead of build_pointer_type), which in turn calls >>>> build_pointer_type_for_mode using the right mode. >>> >>> There seem to be quite a lot of build_pointer_type calls in the C front >>> end (and in c-common.c) that you haven't changed. Could you explain the >>> rule for when a call should or should not be changed, and how it applies >>> to all these calls? >> >> The global approach is to have the same effect as a default >> __attribute__((mode(SI/DImode))) on pointers declared by users so that >> layouts match. That's why only grokdeclarator is changed. >> >> There might be bugs with this approach (e.g. it looks like >> c-common.c:handle_noreturn_attribute doesn't preserve the mode of the >> pointer to function), but my understanding is that they also correspond >> to bugs of __attribute__((mode ())) applied to pointer. The later one >> isn't well tested and one advantage of the VMS port is that it will test >> it much more (as there are many pragma pointer_size in VMS headers). > > So those places would need to change to use build_pointer_type_for_mode as > is done for composite types in c-typeck.c:composite_type, for example? Yes. > I think the patch at least needs a (VMS-specific?) testcase that tests the > new pragma (presuming the testsuite can be run for VMS targets) even if > some cases can't be tested until they are fixed. Argh, that's an issue. We don't run the gcc test suite natively on VMS because there is no port of Dejagnu (if ever doable) to VMS. We haven't tried to test a cross-compiler (and running the executable on the VMS host) because an early attempt for another test suite pointed out slowness and reliability issues. VMS machines could be considered as slow from today's standard POV. I haven't found a method to run only the compile tests and skip the executing one. Is it possible to do that with the gcc test suite ? That's would be very useful to test cross compilers. But I am not worried about the tests of '#pragma pointer_size' per see, as there are very present in system headers and thus simply building a cross-compiler (and much more certainly if Ada is enabled) would thoroughly test it. So I think this patch is test by building the compiler (not now but as soon as I add the builtin macros that protect this pragma in VMS headers). For testing front-end handling of mixed sized pointers, I think we can fallback to the mode __attribute__ and enabling these tests on platforms that support it (mips64/linux, s390x). Is this approach ok to you ? Tristan.
On Thu, 8 Mar 2012, Tristan Gingold wrote: > Argh, that's an issue. We don't run the gcc test suite natively on VMS > because there is no port of Dejagnu (if ever doable) to VMS. We haven't tried > to test a cross-compiler (and running the executable on the VMS host) because > an early attempt for another test suite pointed out slowness and reliability > issues. VMS machines could be considered as slow from today's standard POV. > I haven't found a method to run only the compile tests and skip the executing one. > Is it possible to do that with the gcc test suite ? That's would be very > useful to test cross compilers. Thanks for the explanation. I advise solving the unreliability issues for cross-testing to VMS (and living with the slowness), so that you can run the testsuite, but the patch is OK as-is.
On 03/08/12 05:49, Tristan Gingold wrote: > I haven't found a method to run only the compile tests and skip the executing one. > Is it possible to do that with the gcc test suite ? That's would be very > useful to test cross compilers. Set the "simulator" to be /bin/true. r~
On Mar 8, 2012, at 5:49 AM, Tristan Gingold wrote: > Argh, that's an issue. We don't run the gcc test suite natively on VMS > because there is no port of Dejagnu (if ever doable) to VMS. We haven't tried > to test a cross-compiler (and running the executable on the VMS host) because > an early attempt for another test suite pointed out slowness and reliability > issues. dejagnu slices through this type of testing just fine. dejagnu is also adept at handling reliability issues, its history is littered with unreliability and it is usually fairly easy to work around any unreliability. Selecting targets that happen to be in a `working' state, powercycling them, as needed, noticing when things go wrong, retrying things a few times, as sometimes, something doesn't just work and so on. Also, the cross testing can come in many flavors, you can use a simulator (if you have one) and do cross and test on simulator. You can do this, without the simulator and just fail all the execute tests, you can do canadian cross controlling host to native host testing. As for speed, well, it is all about latency and reliability, the lower the latency and the higher the reliability, the faster the testing, but, it is, what it is. The modern testsuite might be 8 hour range or more, but overnight testing is better than no testing. If you hide it behind a git send hook and stage everything through git and then push out from git as the testsuite passes... you should be able to achieve a nice work-flow. > VMS machines could be considered as slow from today's standard POV. > I haven't found a method to run only the compile tests and skip the executing one. > Is it possible to do that with the gcc test suite ? If you configure a cross compiler and do a make check, you'll just get a fast fail on all the execute tests. If you just look for regressions, you'll notice this works just fine. Sit back, don't worry about the execution failures. When you wire up sim, just say the simulator is /bin/false or /bin/true (set_board_info sim /bin/false) Feel free to email me directly if you need additional pointers. It is fairly easy to setup, though, daunting, one has never done it before.
On Mar 8, 2012, at 7:10 PM, Mike Stump wrote: > On Mar 8, 2012, at 5:49 AM, Tristan Gingold wrote: >> Argh, that's an issue. We don't run the gcc test suite natively on VMS >> because there is no port of Dejagnu (if ever doable) to VMS. We haven't tried >> to test a cross-compiler (and running the executable on the VMS host) because >> an early attempt for another test suite pointed out slowness and reliability >> issues. > > dejagnu slices through this type of testing just fine. dejagnu is also adept at handling reliability issues, its history is littered with unreliability and it is usually fairly easy to work around any unreliability. Selecting targets that happen to be in a `working' state, powercycling them, as needed, noticing when things go wrong, retrying things a few times, as sometimes, something doesn't just work and so on. Also, the cross testing can come in many flavors, you can use a simulator (if you have one) and do cross and test on simulator. You can do this, without the simulator and just fail all the execute tests, you can do canadian cross controlling host to native host testing. As for speed, well, it is all about latency and reliability, the lower the latency and the higher the reliability, the faster the testing, but, it is, what it is. The modern testsuite might be 8 hour range or more, but overnight testing is better than no testing. If you hide it behind a git send hook and stage everything through git and then push out from git as the testsuite passes... you should be able to achieve a nice work-flow. > >> VMS machines could be considered as slow from today's standard POV. >> I haven't found a method to run only the compile tests and skip the executing one. >> Is it possible to do that with the gcc test suite ? > > If you configure a cross compiler and do a make check, you'll just get a fast fail on all the execute tests. If you just look for regressions, you'll notice this works just fine. Sit back, don't worry about the execution failures. When you wire up sim, just say the simulator is /bin/false or /bin/true (set_board_info sim /bin/false) > > Feel free to email me directly if you need additional pointers. It is fairly easy to setup, though, daunting, one has never done it before. Thank you Mike for details. I think I will investigate the cross compiler path first. Tristan.
On Mar 8, 2012, at 4:18 PM, Joseph S. Myers wrote: > On Thu, 8 Mar 2012, Tristan Gingold wrote: > >> Argh, that's an issue. We don't run the gcc test suite natively on VMS >> because there is no port of Dejagnu (if ever doable) to VMS. We haven't tried >> to test a cross-compiler (and running the executable on the VMS host) because >> an early attempt for another test suite pointed out slowness and reliability >> issues. VMS machines could be considered as slow from today's standard POV. >> I haven't found a method to run only the compile tests and skip the executing one. >> Is it possible to do that with the gcc test suite ? That's would be very >> useful to test cross compilers. > > Thanks for the explanation. I advise solving the unreliability issues for > cross-testing to VMS (and living with the slowness), so that you can run > the testsuite, but the patch is OK as-is. Thank you for your comments. No C regressions on x86_64-darwin after full bootstrap. Now committed. Tristan.
On 03/06/12 14:09:23, Tristan Gingold wrote: > The patch is simple: the C front-end will now calls > c_build_pointer_type (instead of build_pointer_type), which in > turn calls build_pointer_type_for_mode using the right mode. [...] Joining this discussion a bit late ... I have a few questions. This change impacts the GUPC branch, mainly because UPC introduces pointers that are generally larger than conventional "C" pointers, and thus some changes were needed in the "build pointer" area, and that logic needed to be adjusted to work with this patch. Here is the new c_build_pointer_type. It a static function constrained to be called from within the c-decl.c file wehre it resides. static tree c_build_pointer_type (tree to_type) { addr_space_t as = to_type == error_mark_node? ADDR_SPACE_GENERIC : TYPE_ADDR_SPACE (to_type); enum machine_mode pointer_mode; if (as != ADDR_SPACE_GENERIC || c_default_pointer_mode == VOIDmode) pointer_mode = targetm.addr_space.pointer_mode (as); else pointer_mode = c_default_pointer_mode; return build_pointer_type_for_mode (to_type, pointer_mode, false); } Here is the old build_pointer_type() in tree.c. It is external/public and is called from various places. tree build_pointer_type (tree to_type) { addr_space_t as = to_type == error_mark_node? ADDR_SPACE_GENERIC : TYPE_ADDR_SPACE (to_type); enum machine_mode pointer_mode = targetm.addr_space.pointer_mode (as); return build_pointer_type_for_mode (to_type, pointer_mode, false); } c_build_pointer_type () introduces c_default_pointer_mode and uses it as long as: as == ADDR_SPACE_GENERIC && c_default_pointer_mode != VOIDmode but build_pointer_type will not use c_default_pointer_mode. My first question is: are there still contexts where build_pointer_type() is called to compile "C" statements/expressions for cases not covered by the calls within c-decl.c? I ask, because we tried moving our checks for UPC pointer-to-shared types from build_pointer_type() into only c_build_pointer_type() and ran into calls to build_pointer_type() that still passed in UPC pointer-to-shared typed objects. What this may indicate is that there are situations where the new c_default_pointer_mode may need to be applied when build_pointer_type() is called. I don't recall off-hand when these situations came up, but it might be easy enough to put some assertions in build_pointer_type() and then run the "C" test suite and see what turns up. Thus, I wonder if it might not be best to generalize build_pointer_type() instead of introducing c_build_pointer_type() and dealing with any "C" specific checks (somehow) there? Thanks, - Gary
On Mar 13, 2012, at 9:57 PM, Gary Funck wrote: > On 03/06/12 14:09:23, Tristan Gingold wrote: >> The patch is simple: the C front-end will now calls >> c_build_pointer_type (instead of build_pointer_type), which in >> turn calls build_pointer_type_for_mode using the right mode. > [...] > > Joining this discussion a bit late ... I have a few questions. > This change impacts the GUPC branch, mainly because UPC introduces > pointers that are generally larger than conventional "C" pointers, > and thus some changes were needed in the "build pointer" area, > and that logic needed to be adjusted to work with this patch. > > Here is the new c_build_pointer_type. It a static function > constrained to be called from within the c-decl.c file wehre > it resides. > > static tree > c_build_pointer_type (tree to_type) > { > addr_space_t as = to_type == error_mark_node? ADDR_SPACE_GENERIC > : TYPE_ADDR_SPACE (to_type); > enum machine_mode pointer_mode; > > if (as != ADDR_SPACE_GENERIC || c_default_pointer_mode == VOIDmode) > pointer_mode = targetm.addr_space.pointer_mode (as); > else > pointer_mode = c_default_pointer_mode; > return build_pointer_type_for_mode (to_type, pointer_mode, false); > } > > Here is the old build_pointer_type() in tree.c. It is external/public > and is called from various places. > > tree > build_pointer_type (tree to_type) > { > addr_space_t as = to_type == error_mark_node? ADDR_SPACE_GENERIC > : TYPE_ADDR_SPACE (to_type); > enum machine_mode pointer_mode = targetm.addr_space.pointer_mode (as); > return build_pointer_type_for_mode (to_type, pointer_mode, false); > } > > c_build_pointer_type () introduces c_default_pointer_mode > and uses it as long as: > as == ADDR_SPACE_GENERIC && c_default_pointer_mode != VOIDmode > but build_pointer_type will not use c_default_pointer_mode. > > My first question is: are there still contexts where build_pointer_type() > is called to compile "C" statements/expressions for cases not > covered by the calls within c-decl.c? Yes. In the discussion, Joseph Myers has pointed them. Some of them are bugs (but should be replaced by build_pointer_type_for_mode), others aren't incorrect (e.g. for built-in functions). Currently c_build_pointer_type is called for user declared pointer types. > I ask, because we tried > moving our checks for UPC pointer-to-shared types from > build_pointer_type() into only c_build_pointer_type() and > ran into calls to build_pointer_type() that still passed > in UPC pointer-to-shared typed objects. What this may > indicate is that there are situations where the new > c_default_pointer_mode may need to be applied when > build_pointer_type() is called. I don't recall off-hand > when these situations came up, but it might be easy enough > to put some assertions in build_pointer_type() and then > run the "C" test suite and see what turns up. I think we should discuss these cases. From my current understanding, these direct calls that affect you are currently bugs. Once I completed the VMS patch submission, I would add some tests to check the handling of pointer modes. > Thus, I wonder if it might not be best to generalize > build_pointer_type() instead of introducing c_build_pointer_type() > and dealing with any "C" specific checks (somehow) there? I am almost sure that this is not correct. build_pointer_type is part of the middle end, where the 'pragma pointer_size' doesn't apply and the ptr_mode should be used. Tristan.
diff --git a/gcc/c-decl.c b/gcc/c-decl.c index de46578..160d393 100644 --- a/gcc/c-decl.c +++ b/gcc/c-decl.c @@ -146,6 +146,10 @@ static int warn_about_return_type; static bool undef_nested_function; +/* Mode used to build pointers (VOIDmode means ptr_mode). */ + +enum machine_mode c_default_pointer_mode = VOIDmode; + /* Each c_binding structure describes one binding of an identifier to a decl. All the decls in a scope - irrespective of namespace - are @@ -558,6 +562,23 @@ add_stmt (tree t) return t; } +/* Build a pointer type using the default pointer mode. */ + +static tree +c_build_pointer_type (tree to_type) +{ + addr_space_t as = to_type == error_mark_node? ADDR_SPACE_GENERIC + : TYPE_ADDR_SPACE (to_type); + enum machine_mode pointer_mode; + + if (as != ADDR_SPACE_GENERIC || c_default_pointer_mode == VOIDmode) + pointer_mode = targetm.addr_space.pointer_mode (as); + else + pointer_mode = c_default_pointer_mode; + return build_pointer_type_for_mode (to_type, pointer_mode, false); +} + + /* Return true if we will want to say something if a goto statement crosses DECL. */ @@ -5683,7 +5704,7 @@ grokdeclarator (const struct c_declarator *declarator, TYPE_NAME (type) = decl; } - type = build_pointer_type (type); + type = c_build_pointer_type (type); /* Process type qualifiers (such as const or volatile) that were given inside the `*'. */ @@ -5918,7 +5939,7 @@ grokdeclarator (const struct c_declarator *declarator, type = TREE_TYPE (type); if (type_quals) type = c_build_qualified_type (type, type_quals); - type = build_pointer_type (type); + type = c_build_pointer_type (type); type_quals = array_ptr_quals; if (type_quals) type = c_build_qualified_type (type, type_quals); @@ -5937,7 +5958,7 @@ grokdeclarator (const struct c_declarator *declarator, "ISO C forbids qualified function types"); if (type_quals) type = c_build_qualified_type (type, type_quals); - type = build_pointer_type (type); + type = c_build_pointer_type (type); type_quals = TYPE_UNQUALIFIED; } else if (type_quals) diff --git a/gcc/c-tree.h b/gcc/c-tree.h index 51c660c..db60935 100644 --- a/gcc/c-tree.h +++ b/gcc/c-tree.h @@ -625,6 +625,10 @@ extern int current_function_returns_abnormally; extern int system_header_p; +/* Mode used to build pointers (VOIDmode means ptr_mode). */ + +extern enum machine_mode c_default_pointer_mode; + /* In c-decl.c */ extern void c_finish_incomplete_decl (tree); extern void c_write_global_declarations (void); diff --git a/gcc/config/vms/vms-c.c b/gcc/config/vms/vms-c.c index 4a2d19c..e4f8493 100644 --- a/gcc/config/vms/vms-c.c +++ b/gcc/config/vms/vms-c.c @@ -27,6 +27,7 @@ along with GCC; see the file COPYING3. If not see #include "tree.h" #include "c-family/c-pragma.h" #include "c-family/c-common.h" +#include "c-tree.h" #include "toplev.h" #include "ggc.h" #include "tm_p.h" @@ -283,6 +284,70 @@ vms_pragma_extern_prefix (cpp_reader * ARG_UNUSED (dummy)) } } +/* #pragma __pointer_size */ + +static enum machine_mode saved_pointer_mode; + +static void +handle_pragma_pointer_size (const char *pragma_name) +{ + enum cpp_ttype tok; + tree x; + + tok = pragma_lex (&x); + if (tok == CPP_NAME) + { + const char *op = IDENTIFIER_POINTER (x); + + if (!strcmp (op, "__save")) + saved_pointer_mode = c_default_pointer_mode; + else if (!strcmp (op, "__restore")) + c_default_pointer_mode = saved_pointer_mode; + else if (!strcmp (op, "__short")) + c_default_pointer_mode = SImode; + else if (!strcmp (op, "__long")) + c_default_pointer_mode = DImode; + else + error ("malformed %<#pragma %s%>, ignoring", pragma_name); + } + else if (tok == CPP_NUMBER) + { + int val; + + if (TREE_CODE (x) == INTEGER_CST) + val = TREE_INT_CST_LOW (x); + else + val = -1; + + if (val == 32) + c_default_pointer_mode = SImode; + else if (val == 64) + c_default_pointer_mode = DImode; + else + error ("invalid constant in %<#pragma %s%>", pragma_name); + } + else + { + error ("malformed %<#pragma %s%>, ignoring", pragma_name); + } +} + +static void +vms_pragma_pointer_size (cpp_reader * ARG_UNUSED (dummy)) +{ + /* Ignore if no -mpointer-size option. */ + if (flag_vms_pointer_size == VMS_POINTER_SIZE_NONE) + return; + + handle_pragma_pointer_size ("pointer_size"); +} + +static void +vms_pragma_required_pointer_size (cpp_reader * ARG_UNUSED (dummy)) +{ + handle_pragma_pointer_size ("required_pointer_size"); +} + /* Add vms-specific pragma. */ void @@ -298,6 +363,10 @@ vms_c_register_pragma (void) vms_pragma_nomember_alignment); c_register_pragma_with_expansion (NULL, "nomember_alignment", vms_pragma_nomember_alignment); + c_register_pragma (NULL, "__pointer_size", + vms_pragma_pointer_size); + c_register_pragma (NULL, "__required_pointer_size", + vms_pragma_required_pointer_size); c_register_pragma (NULL, "__extern_model", vms_pragma_extern_model); c_register_pragma (NULL, "extern_model", vms_pragma_extern_model); c_register_pragma (NULL, "__message", vms_pragma_message);