Re: [C++0x] contiguous bitfields race implementation
On Wed, Aug 31, 2011 at 6:53 PM, Aldy Hernandez wrote: > Sure. I assume in this case, *bit_offset would be 0, right? >>> >>> It would be DECL_FIELD_BIT_OFFSET of that field. Oh, and >>> *byte_offset would be >>> >>> *byte_offset = size_binop (MULT_EXPR, TREE_OPERAND (exp, 2), >>> size_int (DECL_OFFSET_ALIGN >>> (field) / BITS_PER_UNIT)); >>> >>> see expr.c:component_ref_field_offset () (which you conveniently >>> could use here). >>> >>> Note that both TREE_OPERAND (exp, 2) and compoment_ref_field_offset >>> return offsets relative to the immediate containing struct type, not >>> relative to the base object like get_inner_reference does ... >>> (where it is still unclear to me what we are supposed to return from this >>> function ...) > > Ok, I see where your confusion lies. The function is supposed to return a > byte offset from the base object, none of this containing object or > immediate struct, or whatever. Base object, as in "a" in a.i.j.k, as in > what you get back from get_base_address(). > > Originally everything was calculated with get_inner_reference(), which is > relative to the base object, but now we have this hodge podge of > get_inner_reference() calls with ad-hoc calculations and optimizations. > Gladly, we've agreed to use get_inner_reference() and optimize at a later > time. > > So... base object throughout, anything else is a mistake on my part. > > BTW, this whole variable length offset I still can't trigger. I know you > want to cater to Ada, but does it even make sense to enable the C++ memory > model in Ada? Who would ever do this? Be that as it may, I'll humor you > and handle it. > >>> Thus, conservative would be using get_inner_reference here, if the >>> offset is supposed to be relative to the base object. >> >> That said, shouldn't *maxbits not at least make sure to cover the field >> itself? > > Is this what you want? > > /* Be as conservative as possible on variable offsets. */ > if (TREE_OPERAND (exp, 2) > && !host_integerp (TREE_OPERAND (exp, 2), 1)) > { > get_inner_reference (build3 (COMPONENT_REF, > TREE_TYPE (exp), > TREE_OPERAND (exp, 0), > field, NULL_TREE), > &tbitsize, &start_bitpos, &start_offset, > &tmode, &tunsignedp, &tvolatilep, true); > > *byte_offset = start_offset ? start_offset : size_zero_node; > *bit_offset = start_bitpos; > *maxbits = tbitsize; > return; > } Yes, exactly. Richard.
Re: [Ada] Implementation of aspects within generic units
On 31 Aug 2011, at 20:07, Iain Sandoe wrote: On 31 Aug 2011, at 17:34, Arnaud Charlet wrote: In particular I'd be curious to know if revision 178376 has the failure or not. different failure; built with BOOT_CFLAGS="-O0 -g" .. .. it fails debug-compare (ada/exp_ch6.o). There are a lot of seemingly innocuous code differences (nop insertions) masking whatever the real problem is (difficult to compare because of the number of trivial differences). If I touch compare and continue the bootstrap if fails building the native tools with a gnatmake internal error (SYSTEM_ASSERTIONS.ASSERT_FAILURE) namet.adb line 675 -- but that's a target-specific file, right? Sorry, misleading datum... The failure above is the result of building with "-O0 -g" If I build with default BOOT_CFLAGS, 178376 fails in the same wayt as 178381. Iain
Re: [Patch, testsuite] Skip case struct-layout-1 for targets using short enums.
On Thu, Sep 01, 2011 at 02:32:51PM +0800, Terry Guo wrote: > FAIL: gcc.dg/compat/struct-layout-1 c_compat_x_tst.o compile > FAIL: gcc.dg/compat/struct-layout-1 c_compat_y_tst.o compile > UNRESOLVED: gcc.dg/compat/struct-layout-1 c_compat_x_tst.o-c_compat_y_tst.o > link > UNRESOLVED: gcc.dg/compat/struct-layout-1 c_compat_x_tst.o-c_compat_y_tst.o > execute > > On my machine, the error message is "error: width of 'a' exceeds its type". > My GCC is cross built for arm-none-eabi based on upstream GCC 4.6. The > command I run this case is simply as: "make check-gcc > RUNTESTFLAGS="--target_board=arm-none-eabi-qemu/-mthumb/-mcpu=cortex-m3 > compat.exp=struct-layout-1*". Are there anything else I should do to pass > this case? Look into gcc/testsuite/gcc/gcc.log, search for struct-layout-1_generate.exe and see whether -e has been passed to it? If not, debug the tcl bits which are supposed to pass it, but for some reason don't, if yes, look into the generator under debugger why it generates the large enum bitfield bitsizes anyway (you can cut'n'paste the *generate.exe command line, compile it with -g and rerun it under debugger...). Jakub
RE: [Patch, testsuite] Skip case struct-layout-1 for targets using short enums.
Hello Jakub, > > On Thu, Sep 01, 2011 at 09:34:20AM +0800, Terry Guo wrote: > > Here is the patch, is it OK to commit? > > Definitely not. > struct-layout-1* already has support for short enums, see > http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=110247 > So, if that doesn't work for you, you need to debug why. But it still fails for arm-none-eabi as you can see in test results at http://gcc.gnu.org/ml/gcc-testresults/2011-08/msg02739.html. FAIL: gcc.dg/compat/struct-layout-1 c_compat_x_tst.o compile FAIL: gcc.dg/compat/struct-layout-1 c_compat_y_tst.o compile UNRESOLVED: gcc.dg/compat/struct-layout-1 c_compat_x_tst.o-c_compat_y_tst.o link UNRESOLVED: gcc.dg/compat/struct-layout-1 c_compat_x_tst.o-c_compat_y_tst.o execute On my machine, the error message is "error: width of 'a' exceeds its type". My GCC is cross built for arm-none-eabi based on upstream GCC 4.6. The command I run this case is simply as: "make check-gcc RUNTESTFLAGS="--target_board=arm-none-eabi-qemu/-mthumb/-mcpu=cortex-m3 compat.exp=struct-layout-1*". Are there anything else I should do to pass this case? > > > 2011-08-31 Terry Guo > > > > * gcc.dg/compat/struct-layout-1_main.c: Skip the case > > if the target uses short enums. > > > > diff --git a/gcc/testsuite/gcc.dg/compat/struct-layout-1_main.c > > b/gcc/testsuite/gcc.dg/compat/struct-layout-1_main.c > > index b59453e..64275ea 100644 > > --- a/gcc/testsuite/gcc.dg/compat/struct-layout-1_main.c > > +++ b/gcc/testsuite/gcc.dg/compat/struct-layout-1_main.c > > @@ -1,4 +1,5 @@ > > /* { dg-prune-output ".*-Wno-abi.*" } */ > > +/* { dg-skip-if "" { short_enums } } */ > > > >#include "struct-layout-1.h" > > > > Jakub Regards, Terry
Re: [Patch, testsuite] Skip case struct-layout-1 for targets using short enums.
On Thu, Sep 01, 2011 at 09:34:20AM +0800, Terry Guo wrote: > Here is the patch, is it OK to commit? Definitely not. struct-layout-1* already has support for short enums, see http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=110247 So, if that doesn't work for you, you need to debug why. > 2011-08-31 Terry Guo > > * gcc.dg/compat/struct-layout-1_main.c: Skip the case > if the target uses short enums. > > diff --git a/gcc/testsuite/gcc.dg/compat/struct-layout-1_main.c > b/gcc/testsuite/gcc.dg/compat/struct-layout-1_main.c > index b59453e..64275ea 100644 > --- a/gcc/testsuite/gcc.dg/compat/struct-layout-1_main.c > +++ b/gcc/testsuite/gcc.dg/compat/struct-layout-1_main.c > @@ -1,4 +1,5 @@ > /* { dg-prune-output ".*-Wno-abi.*" } */ > +/* { dg-skip-if "" { short_enums } } */ > >#include "struct-layout-1.h" > Jakub
Re: bb partitioning vs optimize_function_for_speed_p
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 08/31/11 15:41, Bernd Schmidt wrote: > On 08/29/11 18:02, Jeff Law wrote: >> On 08/26/11 08:47, Bernd Schmidt wrote: >>> In rest_of_reorder_blocks, we avoid reordering if >>> !optimize_function_for_speed_p. However, we still call >>> insert_section_bounary_note, which can cause problems because >>> now, if we have a sequence of HOT-COLD-HOT blocks, the second set >>> of HOT blocks will end up in the cold section. This causes >>> assembler failures when using exception handling (subtracting >>> labels from different sections). >> >>> Unfortunately, the only way I have of reproducing it is to apply >>> a 67-patch quilt tree backporting the preliminary >>> shrink-wrapping patches to gcc-4.6; then we get >> >>> FAIL: g++.dg/tree-prof/partition2.C compilation, -Os >>> -fprofile-use >> >>> However, the problem is reasonably obvious. Bootstrapped and >>> currently testing in the aforementioned 4.6 tree. Ok for trunk >>> after testing there? >> OK after testing. > > Thanks. Committed, but on second thought something like the below is > probably cleaner; it also avoids partitioning if we're not going to > reorder blocks. Tested along with the shrink-wrapping patches on > i686-linux and mips64-elf. That looks OK too. Jeff -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.11 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iQEcBAEBAgAGBQJOXw67AAoJEBRtltQi2kC78icH/jJAqu9jwiNIRZUgtbCu5f9i gD7S0+BNY/mIooZoQXl6trD9K5Xuzb7Y78DAWR+Zlz10SewyD0+EMWjm+w+z00gb UKLZSH/Qx0sgKXKWw4k72yzPVKCA62D6XskCD7gNA1hbxTp9V+ino31FE0RxaFLM /MpcJqJaKJ72s5kHJq3MRlldY4rcfVWyFHxkTaGgMml3C61TO53o6ZIsC3xiFzUp 2hs/SUq4rJRMV5DMDZiupkcJ7yUpEGLFa5xMQnd8JCKYk4rHkDdP2NGgko5/3szW J7xvjg4EFHkC0befzKyAVWF4eGyUmWUfog/+npvfsSM3rjS1mv126oABAnkwQsU= =CHim -END PGP SIGNATURE-
[Patch, testsuite] Skip case struct-layout-1 for targets using short enums.
Hello, There are some bitfield definitions in this case, such as "enum E2 a:31". For most targets, the ENUM type size equals INT type so this definition is OK. But for some targets like arm_eabi, the default ENUM type size could be the smallest integer type that can contain all of its enumerated values. For this case, the size of variable a is 8 bits. Thus the above definition is regarded as an error. We could fix this issue in two methods: compile the cases with option -fno-short-enums or just skip the case for targets with short enums. For the first one, the user must also provide a library built with option -fno-short-enums, otherwise linking them together could cause subtle issues. This patch prefers the last method. If the no-short-enums library is ready, user can still run this case by performing "make check" with extra option -fno-short-enums. Here is the patch, is it OK to commit? Best regards, Terry 2011-08-31 Terry Guo * gcc.dg/compat/struct-layout-1_main.c: Skip the case if the target uses short enums. diff --git a/gcc/testsuite/gcc.dg/compat/struct-layout-1_main.c b/gcc/testsuite/gcc.dg/compat/struct-layout-1_main.c index b59453e..64275ea 100644 --- a/gcc/testsuite/gcc.dg/compat/struct-layout-1_main.c +++ b/gcc/testsuite/gcc.dg/compat/struct-layout-1_main.c @@ -1,4 +1,5 @@ /* { dg-prune-output ".*-Wno-abi.*" } */ +/* { dg-skip-if "" { short_enums } } */ #include "struct-layout-1.h"
Re: bb partitioning vs optimize_function_for_speed_p
On 08/29/11 18:02, Jeff Law wrote: > On 08/26/11 08:47, Bernd Schmidt wrote: >> In rest_of_reorder_blocks, we avoid reordering if >> !optimize_function_for_speed_p. However, we still call >> insert_section_bounary_note, which can cause problems because now, if >> we have a sequence of HOT-COLD-HOT blocks, the second set of HOT >> blocks will end up in the cold section. This causes assembler >> failures when using exception handling (subtracting labels from >> different sections). > >> Unfortunately, the only way I have of reproducing it is to apply a >> 67-patch quilt tree backporting the preliminary shrink-wrapping >> patches to gcc-4.6; then we get > >> FAIL: g++.dg/tree-prof/partition2.C compilation, -Os -fprofile-use > >> However, the problem is reasonably obvious. Bootstrapped and >> currently testing in the aforementioned 4.6 tree. Ok for trunk after >> testing there? > OK after testing. Thanks. Committed, but on second thought something like the below is probably cleaner; it also avoids partitioning if we're not going to reorder blocks. Tested along with the shrink-wrapping patches on i686-linux and mips64-elf. Bernd * bb-reorder.c (insert_section_boundary_note): Don't check optimize_function_for_speed_p. (gate_handle_partition_blocks): Do it here instead. (gate_handle_reorder_blocks): Move preliminary checks here ... (rest_of_handle_reorder_blocks): ... from here. Index: gcc/bb-reorder.c === --- gcc/bb-reorder.c(revision 178389) +++ gcc/bb-reorder.c(working copy) @@ -1965,8 +1965,7 @@ insert_section_boundary_note (void) rtx new_note; int first_partition = 0; - if (!flag_reorder_blocks_and_partition - || !optimize_function_for_speed_p (cfun)) + if (!flag_reorder_blocks_and_partition) return; FOR_EACH_BB (bb) @@ -2296,7 +2295,17 @@ gate_handle_reorder_blocks (void) { if (targetm.cannot_modify_jumps_p ()) return false; - return (optimize > 0); + /* Don't reorder blocks when optimizing for size because extra jump insns may + be created; also barrier may create extra padding. + + More correctly we should have a block reordering mode that tried to + minimize the combined size of all the jumps. This would more or less + automatically remove extra jumps, but would also try to use more short + jumps instead of long jumps. */ + if (!optimize_function_for_speed_p (cfun)) +return false; + return (optimize > 0 + && (flag_reorder_blocks || flag_reorder_blocks_and_partition)); } @@ -2310,19 +2319,8 @@ rest_of_handle_reorder_blocks (void) splitting possibly introduced more crossjumping opportunities. */ cfg_layout_initialize (CLEANUP_EXPENSIVE); - if ((flag_reorder_blocks || flag_reorder_blocks_and_partition) - /* Don't reorder blocks when optimizing for size because extra jump insns may -be created; also barrier may create extra padding. - -More correctly we should have a block reordering mode that tried to -minimize the combined size of all the jumps. This would more or less -automatically remove extra jumps, but would also try to use more short -jumps instead of long jumps. */ - && optimize_function_for_speed_p (cfun)) -{ - reorder_basic_blocks (); - cleanup_cfg (CLEANUP_EXPENSIVE); -} + reorder_basic_blocks (); + cleanup_cfg (CLEANUP_EXPENSIVE); FOR_EACH_BB (bb) if (bb->next_bb != EXIT_BLOCK_PTR) @@ -2362,6 +2360,9 @@ gate_handle_partition_blocks (void) arises. */ return (flag_reorder_blocks_and_partition && optimize + /* See gate_handle_reorder_blocks. We should not partition if +we are going to omit the reordering. */ + && optimize_function_for_speed_p (cfun) && !DECL_ONE_ONLY (current_function_decl) && !user_defined_section_attribute); }
Re: Copy frame_related bits
On 08/31/11 19:43, Jeff Law wrote: > Presumably the jump & call flags are copied as part of the > shallow_copy_rtx call, thus removing the explicit copy is safe? That's the conclusion I came to. Bernd
Re: Add unwind information to mips epilogues
On 08/31/11 20:43, Richard Sandiford wrote: > Bernd Schmidt writes: >> This is necessary when adding shrink-wrapping; otherwise dwarf2cfi sees >> inconsistent information and aborts. >> >> Tested on mips64-elf together with the rest of the shrink-wrapping >> patches. Ok? > > It looks like the current code doesn't handle the RESTORE instruction. > Could you also test that somehow? A mipsisa32-elf run with -mips16 > ought to work, but some sort of spot-checking of shrink-wrapping + > RESTORE would be fine if that's easier. Will look into that. You mean the mips16e_build_save_restore function? > Also, for the frame_pointer_required case, it looks like there's a > window between the restoration of the frame pointer and the deallocation > of the stack in which the CFA is still defined in terms of the frame > pointer register. Is that significant? If not (e.g. because we > should never need to unwind at that point) then why do we still > update the CFA here: CC'ing rth, as I'm still not terribly familiar with these issues. I've tried to follow alpha.c, which seems to ignore this issue. >> @@ -10324,12 +10330,26 @@ mips_expand_epilogue (bool sibcall_p) >>if (!TARGET_MIPS16) >> target = stack_pointer_rtx; >> >> - emit_insn (gen_add3_insn (target, base, adjust)); >> + insn = emit_insn (gen_add3_insn (target, base, adjust)); >> + if (!frame_pointer_needed && target == stack_pointer_rtx) >> +{ >> + RTX_FRAME_RELATED_P (insn) = 1; >> + add_reg_note (insn, REG_CFA_DEF_CFA, >> +plus_constant (stack_pointer_rtx, step2)); >> +} Here, with !frame_pointer_needed, SP should be the CFA register, so if we modify it we have to use REG_CFA_DEF_CFA. Either that, or I'm confused. > ? If the window is significant, could we avoid it by removing the > !frame_pointer_needed checks from the code above? That causes aborts due to inconsistent information in dwarf2cfi, since we can reach the same instruction with either fp or sp as the CFA register. Bernd
Re: [x86] Use match_test for .md attributes
Uros Bizjak writes: > On Mon, Aug 15, 2011 at 11:57 AM, Richard Sandiford > wrote: > Following on from the two patches I've just posted, this one makes config/i386/*.md use match_test for .md attributes. Tested as described here: >>> http://gcc.gnu.org/ml/gcc-patches/2011-08/msg01182.html >>> * config/i386/i386.md: Use (match_test ...) for attribute tests. * config/i386/mmx.md: Likewise. * config/i386/sse.md: Likewise. >>> >>> - (eq (symbol_ref "TARGET_SSE2") (const_int 0))) >>> + (not (match_test "TARGET_SSE2"))) >>> >>> Jus a question - in predicates.md, i.e. (match_test "!TARGET_SSE2") is >>> used. Do we want to standardize on (not (match_test "...")) form >>> everywhere? >> >> Yeah, good question. I'd used (not (match_test ...)) so that genattrtab >> could better optimise combinations of expressions. I suppose we don't >> yet combine predicate expressions in the same way, so it probably makes >> no difference there. We might use predicate expressions more in future >> though. >> >> I'm happy to convert predicate match_tests at the same time. > > That would be much appreciated. OK, here's what I committed after testing on x86_64-linux-gnu. Richard gcc/ * config/i386/i386.md: Use (match_test ...) for attribute tests. * config/i386/mmx.md: Likewise. * config/i386/sse.md: Likewise. * config/i386/predicates.md (call_insn_operand): Use (not (match_test "...")) instead of (match_test "!...") * config/i386/constraints.md (w): Likewise. Index: gcc/config/i386/i386.md === --- gcc/config/i386/i386.md 2011-08-27 08:17:19.0 +0100 +++ gcc/config/i386/i386.md 2011-08-31 19:13:34.0 +0100 @@ -490,18 +490,16 @@ (define_attr "prefix_0f" "" ;; Set when REX opcode prefix is used. (define_attr "prefix_rex" "" - (cond [(eq (symbol_ref "TARGET_64BIT") (const_int 0)) + (cond [(not (match_test "TARGET_64BIT")) (const_int 0) (and (eq_attr "mode" "DI") (and (eq_attr "type" "!push,pop,call,callv,leave,ibr") (eq_attr "unit" "!mmx"))) (const_int 1) (and (eq_attr "mode" "QI") - (ne (symbol_ref "x86_extended_QIreg_mentioned_p (insn)") - (const_int 0))) + (match_test "x86_extended_QIreg_mentioned_p (insn)")) (const_int 1) -(ne (symbol_ref "x86_extended_reg_mentioned_p (insn)") -(const_int 0)) +(match_test "x86_extended_reg_mentioned_p (insn)") (const_int 1) (and (eq_attr "type" "imovx") (match_operand:QI 1 "ext_QIreg_operand" "")) @@ -551,7 +549,7 @@ (define_attr "modrm" "" (eq_attr "unit" "i387") (const_int 0) (and (eq_attr "type" "incdec") - (and (eq (symbol_ref "TARGET_64BIT") (const_int 0)) + (and (not (match_test "TARGET_64BIT")) (ior (match_operand:SI 1 "register_operand" "") (match_operand:HI 1 "register_operand" "" (const_int 0) @@ -597,7 +595,7 @@ (define_attr "length" "" (attr "length_address"))) (ior (eq_attr "prefix" "vex") (and (eq_attr "prefix" "maybe_vex") - (ne (symbol_ref "TARGET_AVX") (const_int 0 + (match_test "TARGET_AVX"))) (plus (attr "length_vex") (plus (attr "length_immediate") (plus (attr "modrm") @@ -1927,16 +1925,13 @@ (define_insn "*movti_internal_rex64" (set (attr "mode") (cond [(eq_attr "alternative" "2,3") (if_then_else - (ne (symbol_ref "optimize_function_for_size_p (cfun)") - (const_int 0)) + (match_test "optimize_function_for_size_p (cfun)") (const_string "V4SF") (const_string "TI")) (eq_attr "alternative" "4") (if_then_else - (ior (ne (symbol_ref "TARGET_SSE_TYPELESS_STORES") - (const_int 0)) - (ne (symbol_ref "optimize_function_for_size_p (cfun)") - (const_int 0))) + (ior (match_test "TARGET_SSE_TYPELESS_STORES") + (match_test "optimize_function_for_size_p (cfun)")) (const_string "V4SF") (const_string "TI"))] (const_string "DI")))]) @@ -1985,13 +1980,11 @@ (define_insn "*movti_internal_sse" [(set_attr "type" "sselog1,ssemov,ssemov") (set_attr "prefix" "maybe_vex") (set (attr "mode") - (cond [(ior (eq (symbol_ref "TARGET_SSE2") (const_int 0)) - (ne (symbol_ref "optimize_function_for_size_p (cfun)") - (const_int 0))) + (cond [(ior (not (match_test "TARGET_
Re: Vector shuffling
On Wed, 31 Aug 2011, Artem Shinkarov wrote: > On Wed, Aug 31, 2011 at 4:38 PM, Joseph S. Myers > wrote: > > On Wed, 31 Aug 2011, Artem Shinkarov wrote: > > > >> 1) Helper function for the pseudo-builtins. > >> In my case the builtin can have 2 or 3 arguments, and I think that I > >> expressed that in a pretty much short way without any helper function. > >> Am I missing something? > > > > The point is to refactor what's common between this and other > > pseudo-builtins, not to have two pseudo-builtins doing things one way and > > one doing them another way > > Joseph, I don't mind adjusting, just look into the patch and tell me > if the way it is done at the moment is the right way to do it. I don't > see a good reason to write a helper function the way you describe, > because the number of operations we do there is very small. However, > if you think that this is a right way to go, I can put the statements > I am using right now to handle arguments of RID_BUILTIN_SHUFFLE in a > helper function. So is there anything missing? The common parts are I think: * Parse open parenthesis. If there isn't one, a parse error is OK (no worse than at present) but "cannot take address of %<__builtin_whatever%>" would be better. * Parse expression list. * Parse close parenthesis (if not found, parse error). * Check number of arguments against the list of permitted numbers of arguments for this pseudo-builtin, and give an error if the number is wrong. -- Joseph S. Myers jos...@codesourcery.com
[Patch, fortran] [3/4] gfc_ss structs initialization small refactoring: scalars
gfc_ss structs of type GFC_SS_SCALAR (and GFC_SS_REFERENCE) are not uncommon. Let's share their initialization. There is one single case of GFC_SS_REFERENCE, everything else is GFC_SS_SCALAR, so I have decided to always set GFC_SS_SCALAR in the initialisation, and reset it to the right value afterwards in the one case it is needed (in gfc_walk_elemental_function_args). OK? 2011-08-30 Mikael Morin * trans-array.h (gfc_get_scalar_ss): New prototype. * trans-array.c (gfc_get_scalar_ss): New function. (gfc_walk_variable_expr, gfc_walk_op_expr, gfc_walk_elemental_function_args): Re-use gfc_get_scalar_ss. * trans-expr.c (gfc_trans_subarray_assign): Ditto. (gfc_trans_assignment_1): Ditto. * trans-stmt.c (compute_inner_temp_size, gfc_trans_where_assign, gfc_trans_where_3): Ditto. commit 3856ff8a856a3e651e51361bcf8fc260671b Author: Mikael Morin Date: Tue Jun 28 18:05:02 2011 +0200 Factorisation gfc_get_scalar_ss diff --git a/trans-array.c b/trans-array.c index 5f02c87..80a6fe6 100644 --- a/trans-array.c +++ b/trans-array.c @@ -550,6 +550,22 @@ gfc_get_temp_ss (tree type, tree string_length, int dimen) return ss; } + + +/* Creates and initializes a scalar type gfc_ss struct. */ + +gfc_ss * +gfc_get_scalar_ss (gfc_ss *next, gfc_expr *expr) +{ + gfc_ss *ss; + + ss = gfc_get_ss (); + ss->next = next; + ss->type = GFC_SS_SCALAR; + ss->expr = expr; + + return ss; +} /* Free all the SS associated with a loop. */ @@ -7597,17 +7613,8 @@ gfc_walk_variable_expr (gfc_ss * ss, gfc_expr * expr) { if (ref->type == REF_SUBSTRING) { - newss = gfc_get_ss (); - newss->type = GFC_SS_SCALAR; - newss->expr = ref->u.ss.start; - newss->next = ss; - ss = newss; - - newss = gfc_get_ss (); - newss->type = GFC_SS_SCALAR; - newss->expr = ref->u.ss.end; - newss->next = ss; - ss = newss; + ss = gfc_get_scalar_ss (ss, ref->u.ss.start); + ss = gfc_get_scalar_ss (ss, ref->u.ss.end); } /* We're only interested in array sections from now on. */ @@ -7626,13 +7633,7 @@ gfc_walk_variable_expr (gfc_ss * ss, gfc_expr * expr) { case AR_ELEMENT: for (n = ar->dimen + ar->codimen - 1; n >= 0; n--) - { - newss = gfc_get_ss (); - newss->type = GFC_SS_SCALAR; - newss->expr = ar->start[n]; - newss->next = ss; - ss = newss; - } + ss = gfc_get_scalar_ss (ss, ar->start[n]); break; case AR_FULL: @@ -7678,10 +7679,7 @@ gfc_walk_variable_expr (gfc_ss * ss, gfc_expr * expr) case DIMEN_ELEMENT: /* Add SS for elemental (scalar) subscripts. */ gcc_assert (ar->start[n]); - indexss = gfc_get_ss (); - indexss->type = GFC_SS_SCALAR; - indexss->expr = ar->start[n]; - indexss->next = gfc_ss_terminator; + indexss = gfc_get_scalar_ss (gfc_ss_terminator, ar->start[n]); indexss->loop_chain = gfc_ss_terminator; newss->data.info.subscript[n] = indexss; break; @@ -7736,7 +7734,6 @@ gfc_walk_op_expr (gfc_ss * ss, gfc_expr * expr) { gfc_ss *head; gfc_ss *head2; - gfc_ss *newss; head = gfc_walk_subexpr (ss, expr->value.op.op1); if (expr->value.op.op2 == NULL) @@ -7754,8 +7751,6 @@ gfc_walk_op_expr (gfc_ss * ss, gfc_expr * expr) /* One of the operands needs scalarization, the other is scalar. Create a gfc_ss for the scalar expression. */ - newss = gfc_get_ss (); - newss->type = GFC_SS_SCALAR; if (head == ss) { /* First operand is scalar. We build the chain in reverse order, so @@ -7765,17 +7760,13 @@ gfc_walk_op_expr (gfc_ss * ss, gfc_expr * expr) head = head->next; /* Check we haven't somehow broken the chain. */ gcc_assert (head); - newss->next = ss; - head->next = newss; - newss->expr = expr->value.op.op1; + head->next = gfc_get_scalar_ss (ss, expr->value.op.op1); } else /* head2 == head */ { gcc_assert (head2 == head); /* Second operand is scalar. */ - newss->next = head2; - head2 = newss; - newss->expr = expr->value.op.op2; + head2 = gfc_get_scalar_ss (head2, expr->value.op.op2); } return head2; @@ -7830,10 +7821,9 @@ gfc_walk_elemental_function_args (gfc_ss * ss, gfc_actual_arglist *arg, if (newss == head) { /* Scalar argument. */ - newss = gfc_get_ss (); + gcc_assert (type == GFC_SS_SCALAR || type == GFC_SS_REFERENCE); + newss = gfc_get_scalar_ss (head, arg->expr); newss->type = type; - newss->expr = arg->expr; - newss->next = head; } else scalar = 0; d
[Patch, fortran] [4/4] gfc_ss structs initialization small refactoring: minor cleanups
Now that array gfc_ss structs are initialized right after allocation, especially their DIM and DIMEN fields are set properly, later initializations of those fields are now redundant. This patch removes such initializations and/or replaces them with asserts that the value is already correct. OK? 2011-08-30 Mikael Morin * trans-array.c (gfc_trans_constant_array_constructor): Remove superfluous initialisation of DIM field. (gfc_trans_array_constructor): Assert that DIMEN field is properly set. (gfc_conv_expr_descriptor): Ditto. * trans-expr.c (gfc_conv_procedure_call): Ditto. commit 6513df3737d66218914900a23d6ad2c948a986c7 Author: Mikael Morin Date: Tue Jun 28 18:10:33 2011 +0200 Transformation assignements en assertions diff --git a/trans-array.c b/trans-array.c index 80a6fe6..37cdeb5 100644 --- a/trans-array.c +++ b/trans-array.c @@ -1882,7 +1882,6 @@ gfc_trans_constant_array_constructor (gfc_loopinfo * loop, info->start[i] = gfc_index_zero_node; info->end[i] = gfc_index_zero_node; info->stride[i] = gfc_index_one_node; - info->dim[i] = i; } if (info->dimen > loop->temp_dim) @@ -1961,7 +1960,7 @@ gfc_trans_array_constructor (gfc_loopinfo * loop, gfc_ss * ss, locus * where) first_len = true; } - ss->data.info.dimen = loop->dimen; + gcc_assert (ss->data.info.dimen == loop->dimen); c = ss->expr->value.constructor; if (ss->expr->ts.type == BT_CHARACTER) @@ -5915,7 +5914,7 @@ gfc_conv_expr_descriptor (gfc_se * se, gfc_expr * expr, gfc_ss * ss) loop.dimen); se->string_length = loop.temp_ss->string_length; - loop.temp_ss->data.temp.dimen = loop.dimen; + gcc_assert (loop.temp_ss->data.temp.dimen == loop.dimen); loop.temp_ss->data.temp.codimen = loop.codimen; gfc_add_ss_to_loop (&loop, loop.temp_ss); } diff --git a/trans-expr.c b/trans-expr.c index 6a33719..131927c 100644 --- a/trans-expr.c +++ b/trans-expr.c @@ -3576,7 +3576,7 @@ gfc_conv_procedure_call (gfc_se * se, gfc_symbol * sym, /* Set the type of the array. */ tmp = gfc_typenode_for_spec (&comp->ts); - info->dimen = se->loop->dimen; + gcc_assert (info->dimen == se->loop->dimen); /* Evaluate the bounds of the result, if known. */ gfc_set_loop_bounds_from_array_spec (&mapping, se, comp->as); @@ -3611,7 +3611,7 @@ gfc_conv_procedure_call (gfc_se * se, gfc_symbol * sym, /* Set the type of the array. */ tmp = gfc_typenode_for_spec (&ts); - info->dimen = se->loop->dimen; + gcc_assert (info->dimen == se->loop->dimen); /* Evaluate the bounds of the result, if known. */ gfc_set_loop_bounds_from_array_spec (&mapping, se, sym->result->as);
[Patch, fortran] [2/4] gfc_ss structs initialization small refactoring: temps
gfc_ss structs of type GFC_SS_TEMP are not uncommon. Let's share their initialization. OK? 2011-08-30 Mikael Morin * trans-array.h (gfc_get_temp_ss): New prototype. * trans-array.c (gfc_get_temp_ss): New function. (gfc_conv_resolve_dependencies): Re-use gfc_get_temp_ss. (gfc_conv_expr_descriptor): Ditto. * trans-expr.c (gfc_conv_subref_array_arg): Ditto. commit a6d9bcfae8d10cb8c182188dd33fa9953a385792 Author: Mikael Morin Date: Tue Jun 28 17:51:57 2011 +0200 Factorisation gfc_get_temp_ss diff --git a/trans-array.c b/trans-array.c index 107f629..5f02c87 100644 --- a/trans-array.c +++ b/trans-array.c @@ -534,6 +534,24 @@ gfc_get_array_ss (gfc_ss *next, gfc_expr *expr, int dimen, gfc_ss_type type) } +/* Creates and initializes a temporary type gfc_ss struct. */ + +gfc_ss * +gfc_get_temp_ss (tree type, tree string_length, int dimen) +{ + gfc_ss *ss; + + ss = gfc_get_ss (); + ss->next = gfc_ss_terminator; + ss->type = GFC_SS_TEMP; + ss->string_length = string_length; + ss->data.temp.dimen = dimen; + ss->data.temp.type = type; + + return ss; +} + + /* Free all the SS associated with a loop. */ void @@ -3821,13 +3839,9 @@ temporary: if (GFC_ARRAY_TYPE_P (base_type) || GFC_DESCRIPTOR_TYPE_P (base_type)) base_type = gfc_get_element_type (base_type); - loop->temp_ss = gfc_get_ss (); - loop->temp_ss->type = GFC_SS_TEMP; - loop->temp_ss->data.temp.type = base_type; - loop->temp_ss->string_length = dest->string_length; - loop->temp_ss->data.temp.dimen = loop->dimen; + loop->temp_ss = gfc_get_temp_ss (base_type, dest->string_length, + loop->dimen); loop->temp_ss->data.temp.codimen = loop->codimen; - loop->temp_ss->next = gfc_ss_terminator; gfc_add_ss_to_loop (loop, loop->temp_ss); } else @@ -5874,21 +5888,15 @@ gfc_conv_expr_descriptor (gfc_se * se, gfc_expr * expr, gfc_ss * ss) if (need_tmp) { - /* Tell the scalarizer to make a temporary. */ - loop.temp_ss = gfc_get_ss (); - loop.temp_ss->type = GFC_SS_TEMP; - loop.temp_ss->next = gfc_ss_terminator; - - if (expr->ts.type == BT_CHARACTER - && !expr->ts.u.cl->backend_decl) + if (expr->ts.type == BT_CHARACTER && !expr->ts.u.cl->backend_decl) get_array_charlen (expr, se); - loop.temp_ss->data.temp.type = gfc_typenode_for_spec (&expr->ts); - - if (expr->ts.type == BT_CHARACTER) - loop.temp_ss->string_length = expr->ts.u.cl->backend_decl; - else - loop.temp_ss->string_length = NULL; + /* Tell the scalarizer to make a temporary. */ + loop.temp_ss = gfc_get_temp_ss (gfc_typenode_for_spec (&expr->ts), + ((expr->ts.type == BT_CHARACTER) + ? expr->ts.u.cl->backend_decl + : NULL), + loop.dimen); se->string_length = loop.temp_ss->string_length; loop.temp_ss->data.temp.dimen = loop.dimen; diff --git a/trans-array.h b/trans-array.h index 26d02ec..e2718b2 100644 --- a/trans-array.h +++ b/trans-array.h @@ -89,6 +89,8 @@ void gfc_mark_ss_chain_used (gfc_ss *, unsigned); void gfc_free_ss_chain (gfc_ss *); /* Allocate a new array type ss. */ gfc_ss *gfc_get_array_ss (gfc_ss *, gfc_expr *, int, gfc_ss_type); +/* Allocate a new temporary type ss. */ +gfc_ss *gfc_get_temp_ss (tree, tree, int); /* Calculates the lower bound and stride of array sections. */ void gfc_conv_ss_startstride (gfc_loopinfo *); diff --git a/trans-expr.c b/trans-expr.c index 04cf4dd..0e85060 100644 --- a/trans-expr.c +++ b/trans-expr.c @@ -2395,18 +2395,12 @@ gfc_conv_subref_array_arg (gfc_se * parmse, gfc_expr * expr, int g77, || GFC_DESCRIPTOR_TYPE_P (base_type)) base_type = gfc_get_element_type (base_type); - loop.temp_ss = gfc_get_ss ();; - loop.temp_ss->type = GFC_SS_TEMP; - loop.temp_ss->data.temp.type = base_type; - - if (expr->ts.type == BT_CHARACTER) -loop.temp_ss->string_length = expr->ts.u.cl->backend_decl; - else -loop.temp_ss->string_length = NULL; + loop.temp_ss = gfc_get_temp_ss (base_type, ((expr->ts.type == BT_CHARACTER) + ? expr->ts.u.cl->backend_decl + : NULL), + loop.dimen); parmse->string_length = loop.temp_ss->string_length; - loop.temp_ss->data.temp.dimen = loop.dimen; - loop.temp_ss->next = gfc_ss_terminator; /* Associate the SS with the loop. */ gfc_add_ss_to_loop (&loop, loop.temp_ss);
[Patch, fortran] [1/4] gfc_ss structs initialization small refactoring: arrays
All the gfc_ss of type GFC_SS_FUNCTION, GFC_SS_ARRAY, GFC_SS_CONSTRUCTOR, GFC_SS_VECTOR, ... have the same kind of initialization. Let's share it. OK? 2011-08-30 Mikael Morin * trans-array.h (gfc_get_array_ss): New prototype. * trans-array.c (gfc_get_array_ss): New function. (gfc_walk_variable_expr, gfc_walk_function_expr, gfc_walk_array_constructor): Re-use gfc_get_array_ss. * trans-expr.c (gfc_trans_subarray_assign): Ditto. * trans-intrinsic.c (gfc_walk_intrinsic_bound, gfc_walk_intrinsic_libfunc): Ditto. * trans-io.c (transfer_array_component): Ditto. commit e629d3cf536d1b8b81fecd2b7f152b83b3a09029 Author: Mikael Morin Date: Tue Jun 28 15:16:47 2011 +0200 Factorisation gfc_get_array_ss diff --git a/trans-array.c b/trans-array.c index 6dc1e17..107f629 100644 --- a/trans-array.c +++ b/trans-array.c @@ -511,6 +511,29 @@ gfc_free_ss (gfc_ss * ss) } +/* Creates and initializes an array type gfc_ss struct. */ + +gfc_ss * +gfc_get_array_ss (gfc_ss *next, gfc_expr *expr, int dimen, gfc_ss_type type) +{ + gfc_ss *ss; + gfc_ss_info *info; + int i; + + ss = gfc_get_ss (); + ss->next = next; + ss->type = type; + ss->expr = expr; + info = &ss->data.info; + info->dimen = dimen; + info->codimen = 0; + for (i = 0; i < info->dimen; i++) +info->dim[i] = i; + + return ss; +} + + /* Free all the SS associated with a loop. */ void @@ -7605,12 +7628,7 @@ gfc_walk_variable_expr (gfc_ss * ss, gfc_expr * expr) break; case AR_FULL: - newss = gfc_get_ss (); - newss->type = GFC_SS_SECTION; - newss->expr = expr; - newss->next = ss; - newss->data.info.dimen = ar->as->rank; - newss->data.info.codimen = 0; + newss = gfc_get_array_ss (ss, expr, ar->as->rank, GFC_SS_SECTION); newss->data.info.ref = ref; /* Make sure array is the same as array(:,:), this way @@ -7619,7 +7637,6 @@ gfc_walk_variable_expr (gfc_ss * ss, gfc_expr * expr) ar->codimen = 0; for (n = 0; n < ar->dimen; n++) { - newss->data.info.dim[n] = n; ar->dimen_type[n] = DIMEN_RANGE; gcc_assert (ar->start[n] == NULL); @@ -7638,15 +7655,10 @@ gfc_walk_variable_expr (gfc_ss * ss, gfc_expr * expr) break; case AR_SECTION: - newss = gfc_get_ss (); - newss->type = GFC_SS_SECTION; - newss->expr = expr; - newss->next = ss; - newss->data.info.dimen = 0; - newss->data.info.codimen = 0; + newss = gfc_get_array_ss (ss, expr, 0, GFC_SS_SECTION); newss->data.info.ref = ref; - /* We add SS chains for all the subscripts in the section. */ + /* We add SS chains for all the subscripts in the section. */ for (n = 0; n < ar->dimen + ar->codimen; n++) { gfc_ss *indexss; @@ -7678,10 +7690,8 @@ gfc_walk_variable_expr (gfc_ss * ss, gfc_expr * expr) case DIMEN_VECTOR: /* Create a GFC_SS_VECTOR index in which we can store the vector's descriptor. */ - indexss = gfc_get_ss (); - indexss->type = GFC_SS_VECTOR; - indexss->expr = ar->start[n]; - indexss->next = gfc_ss_terminator; + indexss = gfc_get_array_ss (gfc_ss_terminator, ar->start[n], + 1, GFC_SS_VECTOR); indexss->loop_chain = gfc_ss_terminator; newss->data.info.subscript[n] = indexss; newss->data.info.dim[newss->data.info.dimen @@ -7852,11 +7862,9 @@ gfc_walk_elemental_function_args (gfc_ss * ss, gfc_actual_arglist *arg, static gfc_ss * gfc_walk_function_expr (gfc_ss * ss, gfc_expr * expr) { - gfc_ss *newss; gfc_intrinsic_sym *isym; gfc_symbol *sym; gfc_component *comp = NULL; - int n; isym = expr->value.function.isym; @@ -7872,16 +7880,7 @@ gfc_walk_function_expr (gfc_ss * ss, gfc_expr * expr) gfc_is_proc_ptr_comp (expr, &comp); if ((!comp && gfc_return_by_reference (sym) && sym->result->attr.dimension) || (comp && comp->attr.dimension)) -{ - newss = gfc_get_ss (); - newss->type = GFC_SS_FUNCTION; - newss->expr = expr; - newss->next = ss; - newss->data.info.dimen = expr->rank; - for (n = 0; n < newss->data.info.dimen; n++) - newss->data.info.dim[n] = n; - return newss; -} +return gfc_get_array_ss (ss, expr, expr->rank, GFC_SS_FUNCTION); /* Walk the parameters of an elemental function. For now we always pass by reference. */ @@ -7900,18 +7899,7 @@ gfc_walk_function_expr (gfc_ss * ss, gfc_expr * expr) static gfc_ss * gfc_walk_array_constructor (gfc_ss * ss, gfc_expr * expr) { - gfc_ss *newss; - int n; - - newss = gfc_get_ss (); - newss->type = GFC_SS_CONSTRUCTOR; - newss->expr = expr; - newss->next
[Patch, fortran] [0/4] gfc_ss structs initialization small refactoring
Hello, the 4 follow-up patches try to refactor some common code initializing gfc_ss structs. Regression-tested (the 4 patches together only) on x86_64-freebsd8.2. OK for trunk? Mikael trans-array.c | 181 - trans-array.h |6 ++ trans-expr.c | 45 - trans-intrinsic.c | 24 +--- trans-io.c|8 +-- trans-stmt.c | 30 +++--- 6 files changed, 114 insertions(+), 180 deletions(-)
Re: [Ada] Implementation of aspects within generic units
On 31 Aug 2011, at 20:07, Iain Sandoe wrote: Same for revision 178311 will set this going .. prob. tomorrow before a report. fails with: "exp_light.ali" not found "exp_light.adb" must be compiled (that issue was already reported by Richi). so .. not much progress ... will try bisecting from 311 -> 261 which did bootstrap for me.. cheers Iain
Re: [Ada] Implementation of aspects within generic units
On 31 Aug 2011, at 17:34, Arnaud Charlet wrote: Program received signal EXC_BAD_ACCESS, Could not access memory. Reason: KERN_PROTECTION_FAILURE at address: 0x 0x001c4fa0 in lib__writ__write_ali () (gdb) bt #0 0x001c4fa0 in lib__writ__write_ali () #1 0x0036799a in _ada_gnat1drv () #2 0x000305f5 in gnat_parse_file () I just triple checked, and revision 178381 is OK for me on x86_64-unknown-linux-gnu. Unfortunately without debug info, the above traceback isn't giving much info. Just a shot in the dark, can you try to pinpoint at which revision things started to break? That'd be useful. In particular I'd be curious to know if revision 178376 has the failure or not. different failure; built with BOOT_CFLAGS="-O0 -g" .. .. it fails debug-compare (ada/exp_ch6.o). There are a lot of seemingly innocuous code differences (nop insertions) masking whatever the real problem is (difficult to compare because of the number of trivial differences). If I touch compare and continue the bootstrap if fails building the native tools with a gnatmake internal error (SYSTEM_ASSERTIONS.ASSERT_FAILURE) namet.adb line 675 -- but that's a target-specific file, right? === ... FWIW, I find debugging ada bootstrap problems especially intractable (any hints/advice on good techniques would be most welcome) I've yet to succeed in getting powerpc-darwin9 (trunk) to bootstrap .. :-( (although with some jiggery-pokery it is possible to bootstrap 4.6 on it.) Same for revision 178311 will set this going .. prob. tomorrow before a report. Also - I don't know that Darwin is the best platform for test here -- it's probably the least exercised ... so other people might wish to chime in cheers Iain
Re: Add unwind information to mips epilogues
Bernd Schmidt writes: > This is necessary when adding shrink-wrapping; otherwise dwarf2cfi sees > inconsistent information and aborts. > > Tested on mips64-elf together with the rest of the shrink-wrapping > patches. Ok? It looks like the current code doesn't handle the RESTORE instruction. Could you also test that somehow? A mipsisa32-elf run with -mips16 ought to work, but some sort of spot-checking of shrink-wrapping + RESTORE would be fine if that's easier. Also, for the frame_pointer_required case, it looks like there's a window between the restoration of the frame pointer and the deallocation of the stack in which the CFA is still defined in terms of the frame pointer register. Is that significant? If not (e.g. because we should never need to unwind at that point) then why do we still update the CFA here: > @@ -10324,12 +10330,26 @@ mips_expand_epilogue (bool sibcall_p) >if (!TARGET_MIPS16) > target = stack_pointer_rtx; > > - emit_insn (gen_add3_insn (target, base, adjust)); > + insn = emit_insn (gen_add3_insn (target, base, adjust)); > + if (!frame_pointer_needed && target == stack_pointer_rtx) > + { > + RTX_FRAME_RELATED_P (insn) = 1; > + add_reg_note (insn, REG_CFA_DEF_CFA, > + plus_constant (stack_pointer_rtx, step2)); > + } and here: >/* Copy TARGET into the stack pointer. */ >if (target != stack_pointer_rtx) > -mips_emit_move (stack_pointer_rtx, target); > +{ > + insn = mips_emit_move (stack_pointer_rtx, target); > + if (!frame_pointer_needed) > + { > + add_reg_note (insn, REG_CFA_DEF_CFA, > + plus_constant (stack_pointer_rtx, step2)); > + RTX_FRAME_RELATED_P (insn) = 1; > + } > +} ? If the window is significant, could we avoid it by removing the !frame_pointer_needed checks from the code above? Richard
Re: [C++0x] contiguous bitfields race implementation
Did you test Ada and enable the C++ memory model? ;) See my earlier comment on Ada. Who would ever use the C++ memory model on Ada? Btw, even if the bitfield we access (and thus the whole region) is at a constant offset, the field _following_ the bitregion (the one you query above with get_inner_reference) can be at variable offset. I suggest to simply not include any padding in that case (which would be, TREE_CODE (DECL_FIELD_OFFSET (fld)) != INTEGER_CST). I still have not found a place where we get a variable offset here (after folding the computation). How about we put a gcc_assert() along with a big fat comment with your above suggestion when we encounter this. Or can you give me an example of this case? Is what you want, that we call get_inner_reference once, and then use DECL_FIELD_OFFSET+DECL_FIELD_BIT_OFFSET to calculate any subsequent bit offset? I found this to be quite tricky with padding, and such, but am willing to give it a whirl again. Yes. I have added a comment to this effect, and will address it along with the get_inner_reference() removal you have suggested as a followup.
Re: [C++0x] contiguous bitfields race implementation
Sure. I assume in this case, *bit_offset would be 0, right? It would be DECL_FIELD_BIT_OFFSET of that field. Oh, and *byte_offset would be *byte_offset = size_binop (MULT_EXPR, TREE_OPERAND (exp, 2), size_int (DECL_OFFSET_ALIGN (field) / BITS_PER_UNIT)); see expr.c:component_ref_field_offset () (which you conveniently could use here). Note that both TREE_OPERAND (exp, 2) and compoment_ref_field_offset return offsets relative to the immediate containing struct type, not relative to the base object like get_inner_reference does ... (where it is still unclear to me what we are supposed to return from this function ...) Ok, I see where your confusion lies. The function is supposed to return a byte offset from the base object, none of this containing object or immediate struct, or whatever. Base object, as in "a" in a.i.j.k, as in what you get back from get_base_address(). Originally everything was calculated with get_inner_reference(), which is relative to the base object, but now we have this hodge podge of get_inner_reference() calls with ad-hoc calculations and optimizations. Gladly, we've agreed to use get_inner_reference() and optimize at a later time. So... base object throughout, anything else is a mistake on my part. BTW, this whole variable length offset I still can't trigger. I know you want to cater to Ada, but does it even make sense to enable the C++ memory model in Ada? Who would ever do this? Be that as it may, I'll humor you and handle it. Thus, conservative would be using get_inner_reference here, if the offset is supposed to be relative to the base object. That said, shouldn't *maxbits not at least make sure to cover the field itself? Is this what you want? /* Be as conservative as possible on variable offsets. */ if (TREE_OPERAND (exp, 2) && !host_integerp (TREE_OPERAND (exp, 2), 1)) { get_inner_reference (build3 (COMPONENT_REF, TREE_TYPE (exp), TREE_OPERAND (exp, 0), field, NULL_TREE), &tbitsize, &start_bitpos, &start_offset, &tmode, &tunsignedp, &tvolatilep, true); *byte_offset = start_offset ? start_offset : size_zero_node; *bit_offset = start_bitpos; *maxbits = tbitsize; return; }
Initial shrink-wrapping patch
This is a new version of the original 4/6 shrink wrapping patch, minus the preliminary bits that were already approved, and with some extra bug fixes that were discovered in the meantime. This is now mostly contained to just function.c. I'll resubmit the other parts (i.e. exposing more shrink-wrapping opportunities through code movement and copy propagation) later after retesting them. This requires the two patches I sent earlier today, for mips epilogue unwinding and copying the frame_related bit. Together with these, BSRT on i686-linux, and sim tested on mips64-elf. ARM tests are still running so I've dropped those target-specific bits for now. Experience with testing on mips shows that at the very least the consistency checks in dwarf2cfi are working well. Bernd * doc/tm.texi (RETURN_ADDR_REGNUM): Document. * doc/invoke.texi (-fshrink-wrap): Document. * opts.c (default_options_table): Add it. * common.opt (fshrink-wrap): Add. * function.c (emit_return_into_block): Remove useless declaration. (record_hard_reg_uses_1, record_hard_reg_uses, frame_required_for_rtx, requires_stack_frame_p, gen_return_pattern): New static functions. (emit_return_into_block): New arg simple_p. All callers changed. Use gen_return_pattern. (thread_prologue_and_epilogue_insns): Implement shrink-wrapping. * config/i386/i386.md (returns): New code_iterator. (return_str, return_cond): New code_attrs. (return, return_internal, return_internal_long, return_pop_internal, return_indirect_internal): New patterns, replacing... (return, return_internal, return_internal_long, return_pop_internal, return_indirect_internal): ... these. * config/i386/i386.c (ix86_expand_epilogue): Adjust to expand simple returns. (ix86_pad_returns): Likewise. Index: gcc/doc/tm.texi === --- gcc/doc/tm.texi (revision 178135) +++ gcc/doc/tm.texi (working copy) @@ -3208,6 +3208,12 @@ Define this if the return address of a p from the frame pointer of the previous stack frame. @end defmac +@defmac RETURN_ADDR_REGNUM +If defined, a C expression whose value is the register number of the return +address for the current function. Targets that pass the return address on +the stack should not define this macro. +@end defmac + @defmac INCOMING_RETURN_ADDR_RTX A C expression whose value is RTL representing the location of the incoming return address at the beginning of any function, before the Index: gcc/doc/invoke.texi === --- gcc/doc/invoke.texi (revision 178135) +++ gcc/doc/invoke.texi (working copy) @@ -395,10 +395,10 @@ Objective-C and Objective-C++ Dialects}. -fschedule-insns -fschedule-insns2 -fsection-anchors @gol -fselective-scheduling -fselective-scheduling2 @gol -fsel-sched-pipelining -fsel-sched-pipelining-outer-loops @gol --fsignaling-nans -fsingle-precision-constant -fsplit-ivs-in-unroller @gol --fsplit-wide-types -fstack-protector -fstack-protector-all @gol --fstrict-aliasing -fstrict-overflow -fthread-jumps -ftracer @gol --ftree-bit-ccp @gol +-fshrink-wrap -fsignaling-nans -fsingle-precision-constant @gol +-fsplit-ivs-in-unroller -fsplit-wide-types -fstack-protector @gol +-fstack-protector-all -fstrict-aliasing -fstrict-overflow @gol +-fthread-jumps -ftracer -ftree-bit-ccp @gol -ftree-builtin-call-dce -ftree-ccp -ftree-ch -ftree-copy-prop @gol -ftree-copyrename -ftree-dce -ftree-dominator-opts -ftree-dse @gol -ftree-forwprop -ftree-fre -ftree-loop-if-convert @gol @@ -6872,6 +6872,12 @@ This option has no effect until one of @ When pipelining loops during selective scheduling, also pipeline outer loops. This option has no effect until @option{-fsel-sched-pipelining} is turned on. +@item -fshrink-wrap +@opindex fshrink-wrap +Emit function prologues only before parts of the function that need it, +rather than at the top of the function. This flag is enabled by default at +@option{-O} and higher. + @item -fcaller-saves @opindex fcaller-saves Enable values to be allocated in registers that will be clobbered by Index: gcc/opts.c === --- gcc/opts.c (revision 178135) +++ gcc/opts.c (working copy) @@ -434,6 +434,7 @@ static const struct default_options defa { OPT_LEVELS_1_PLUS, OPT_fipa_reference, NULL, 1 }, { OPT_LEVELS_1_PLUS, OPT_fipa_profile, NULL, 1 }, { OPT_LEVELS_1_PLUS, OPT_fmerge_constants, NULL, 1 }, +{ OPT_LEVELS_1_PLUS, OPT_fshrink_wrap, NULL, 1 }, { OPT_LEVELS_1_PLUS, OPT_fsplit_wide_types, NULL, 1 }, { OPT_LEVELS_1_PLUS, OPT_ftree_ccp, NULL, 1 }, { OPT_LEVELS_1_PLUS, OPT_ftree_bit_ccp, NULL, 1 }, Index: gcc/function.c === --- gcc/function.c (revision 178135) +++ gcc/fu
Re: Copy frame_related bits
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 08/31/11 11:20, Bernd Schmidt wrote: > With the final shrink-wrapping patch applied, I see failures in > dwarf2cfi on mips64-elf. The problem is that reorg.c uses copy_rtx > to copy instructions, and for some reason that clears the > frame_related bit. We end up with a prologue insn in a delay slot, > and dwarf2cfi disregards its effects. > > I see no reason to do this, and testing (BSRT i686-linux, plus > mips64-elf sim testing) showed no reason either. Ok? Presumably the jump & call flags are copied as part of the shallow_copy_rtx call, thus removing the explicit copy is safe? Based on my review of the history of that code I don't think ignoring the frame related flag was ever intentional... OK. jeff -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.11 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iQEcBAEBAgAGBQJOXnLcAAoJEBRtltQi2kC735MH/AsvPSEjsFpvRebGyVGhUSGL A4nFRZjXVuRDpUFwRrWKGdqLPPdqzdJ5ixaS+Z1pHDwXYwaLXO46XRldDf8GbcGT Dl2tCZSel9KTVduEm4wH49ZXTMn4UZBLbpIfhO19SUXZ399AmzIeBCvfMtGPJczM fZNjFI+/CsNQ9n6GsWMit0qjYqSZCTyVabRCKsGHiMRmnA8WtXfhdEZR5ZSANqo6 98HgCtNkD1Q643Sc9SNqXbIDdZF1CB1hKUl01Rf+SADX6In4SrDOXGYgmvt/plDB b8j1zt0Agmjpw36f4dgIeaF50WzU1FpeaxZYZ8hfFcwBY6mBenDXB0lIGg/EHx0= =iOxe -END PGP SIGNATURE-
Re: Vector shuffling
On Aug 31, 2011, at 1:27 AM, Artem Shinkarov wrote: >> If you're going to add vector shuffling builtins, you might consider adding >> the same builtin that clang has for compatibility: >> http://clang.llvm.org/docs/LanguageExtensions.html#__builtin_shufflevector >> >> It should be straight-forward to map it into the same IR. >> >> -Chris >> > > Chris > > I am trying to use OpenCL syntax here which says that the mask for > shuffling is a vector. Also I didn't really get from the clang > description if the indexes could be non-constnants? If not, then I > have a problem here, because I want to support this. Yes, constant elements are required for this builtin. It is an implementation detail, but Clang doesn't implement the OpenCL shuffle operations with builtins. -Chris
Add unwind information to mips epilogues
This is necessary when adding shrink-wrapping; otherwise dwarf2cfi sees inconsistent information and aborts. Tested on mips64-elf together with the rest of the shrink-wrapping patches. Ok? Bernd * config/mips/mips.c (cfa_restores): New static variable. (mips_restore_reg): Add to it. (mips_expand_epilogue): Initialize it. Annotate RTL to ensure dwarf2cfi sees the effects of the epilogue. Index: gcc/config/mips/mips.c === --- gcc/config/mips/mips.c (revision 178135) +++ gcc/config/mips/mips.c (working copy) @@ -10227,7 +10227,10 @@ mips_expand_prologue (void) emit_insn (gen_blockage ()); } -/* Emit instructions to restore register REG from slot MEM. */ +static rtx cfa_restores = NULL_RTX; + +/* Emit instructions to restore register REG from slot MEM. Also update + the cfa_restores list. */ static void mips_restore_reg (rtx reg, rtx mem) @@ -10238,6 +10241,7 @@ mips_restore_reg (rtx reg, rtx mem) reg = gen_rtx_REG (GET_MODE (reg), GP_REG_FIRST + 7); mips_emit_save_slot_move (reg, mem, MIPS_EPILOGUE_TEMP (GET_MODE (reg))); + cfa_restores = alloc_reg_note (REG_CFA_RESTORE, reg, cfa_restores); } /* Emit any instructions needed before a return. */ @@ -10268,6 +10272,8 @@ mips_expand_epilogue (bool sibcall_p) HOST_WIDE_INT step1, step2; rtx base, target, insn; + cfa_restores = NULL_RTX; + if (!sibcall_p && mips_can_use_return_insn ()) { emit_jump_insn (gen_return ()); @@ -10324,12 +10330,26 @@ mips_expand_epilogue (bool sibcall_p) if (!TARGET_MIPS16) target = stack_pointer_rtx; - emit_insn (gen_add3_insn (target, base, adjust)); + insn = emit_insn (gen_add3_insn (target, base, adjust)); + if (!frame_pointer_needed && target == stack_pointer_rtx) + { + RTX_FRAME_RELATED_P (insn) = 1; + add_reg_note (insn, REG_CFA_DEF_CFA, + plus_constant (stack_pointer_rtx, step2)); + } } /* Copy TARGET into the stack pointer. */ if (target != stack_pointer_rtx) -mips_emit_move (stack_pointer_rtx, target); +{ + insn = mips_emit_move (stack_pointer_rtx, target); + if (!frame_pointer_needed) + { + add_reg_note (insn, REG_CFA_DEF_CFA, + plus_constant (stack_pointer_rtx, step2)); + RTX_FRAME_RELATED_P (insn) = 1; + } +} /* If we're using addressing macros, $gp is implicitly used by all SYMBOL_REFs. We must emit a blockage insn before restoring $gp @@ -10393,9 +10413,14 @@ mips_expand_epilogue (bool sibcall_p) /* If we don't use shoadow register set, we need to update SP. */ if (!cfun->machine->use_shadow_register_set_p && step2 > 0) - emit_insn (gen_add3_insn (stack_pointer_rtx, - stack_pointer_rtx, - GEN_INT (step2))); + { + insn = emit_insn (gen_add3_insn (stack_pointer_rtx, + stack_pointer_rtx, + GEN_INT (step2))); + REG_NOTES (insn) = cfa_restores; + RTX_FRAME_RELATED_P (insn) = 1; + add_reg_note (insn, REG_CFA_DEF_CFA, stack_pointer_rtx); + } /* Move to COP0 Status. */ emit_insn (gen_cop0_move (gen_rtx_REG (SImode, COP0_STATUS_REG_NUM), @@ -10405,9 +10430,14 @@ mips_expand_epilogue (bool sibcall_p) { /* Deallocate the final bit of the frame. */ if (step2 > 0) - emit_insn (gen_add3_insn (stack_pointer_rtx, - stack_pointer_rtx, - GEN_INT (step2))); + { + insn = emit_insn (gen_add3_insn (stack_pointer_rtx, + stack_pointer_rtx, + GEN_INT (step2))); + REG_NOTES (insn) = cfa_restores; + RTX_FRAME_RELATED_P (insn) = 1; + add_reg_note (insn, REG_CFA_DEF_CFA, stack_pointer_rtx); + } } }
Re: Vector shuffling
On Wed, Aug 31, 2011 at 4:38 PM, Joseph S. Myers wrote: > On Wed, 31 Aug 2011, Artem Shinkarov wrote: > >> 1) Helper function for the pseudo-builtins. >> In my case the builtin can have 2 or 3 arguments, and I think that I >> expressed that in a pretty much short way without any helper function. >> Am I missing something? > > The point is to refactor what's common between this and other > pseudo-builtins, not to have two pseudo-builtins doing things one way and > one doing them another way Joseph, I don't mind adjusting, just look into the patch and tell me if the way it is done at the moment is the right way to do it. I don't see a good reason to write a helper function the way you describe, because the number of operations we do there is very small. However, if you think that this is a right way to go, I can put the statements I am using right now to handle arguments of RID_BUILTIN_SHUFFLE in a helper function. So is there anything missing? Thanks, Artem. > -- > Joseph S. Myers > jos...@codesourcery.com >
Copy frame_related bits
With the final shrink-wrapping patch applied, I see failures in dwarf2cfi on mips64-elf. The problem is that reorg.c uses copy_rtx to copy instructions, and for some reason that clears the frame_related bit. We end up with a prologue insn in a delay slot, and dwarf2cfi disregards its effects. I see no reason to do this, and testing (BSRT i686-linux, plus mips64-elf sim testing) showed no reason either. Ok? Bernd * rtl.c (copy_rtx): Do not handle frame_related, jump or call flags specially. Index: gcc/rtl.c === --- gcc/rtl.c (revision 178135) +++ gcc/rtl.c (working copy) @@ -289,12 +289,6 @@ copy_rtx (rtx orig) walks over the RTL. */ RTX_FLAG (copy, used) = 0; - /* We do not copy FRAME_RELATED for INSNs. */ - if (INSN_P (orig)) -RTX_FLAG (copy, frame_related) = 0; - RTX_FLAG (copy, jump) = RTX_FLAG (orig, jump); - RTX_FLAG (copy, call) = RTX_FLAG (orig, call); - format_ptr = GET_RTX_FORMAT (GET_CODE (copy)); for (i = 0; i < GET_RTX_LENGTH (GET_CODE (copy)); i++)
Re: [Ada] Implementation of aspects within generic units
> Program received signal EXC_BAD_ACCESS, Could not access memory. > Reason: KERN_PROTECTION_FAILURE at address: 0x > 0x001c4fa0 in lib__writ__write_ali () > (gdb) bt > #0 0x001c4fa0 in lib__writ__write_ali () > #1 0x0036799a in _ada_gnat1drv () > #2 0x000305f5 in gnat_parse_file () I just triple checked, and revision 178381 is OK for me on x86_64-unknown-linux-gnu. Unfortunately without debug info, the above traceback isn't giving much info. Just a shot in the dark, can you try to pinpoint at which revision things started to break? That'd be useful. In particular I'd be curious to know if revision 178376 has the failure or not. Same for revision 178311
[RFC] Split -mrecip
Hello, I'd like to have tighter control over the individual situations that -mrecip handles, and I think the user might appreciate this too. Hence I've introduced four new target options -mrecip-div, -mrecip-sqrt, -mrecip-vec-div and -mrecip-vec-sqrt. I've redefined -mrecip to be equivalent to using those four options together. In addition one can selectively disable some part via -mrecip -mno-recip-vec for instance. I was split mind about the approach, I could also have done like rs6000 (-mrecip=) with the disadvantage of having to write an own parser as our opt framework can't deal with comma separated lists of masks. With the approach I chose our opt framework gets most of the work done. I've decided to not use four new bits from target_flags, and instead created a new mask (recip_mask). Four bits would have fit in target bits right now, but in the future we might want to add more specialization, like modes for which the reciprocals are active. What do you think? Ciao, Michael. * i386/i386.opt (recip_mask_explicit, x_recip_mask_explicit): New variable and cl_target member. (mrecip-div, mrecip-sqrt, mrecip-vec-div, mrecip-vec-sqrt): New options. * common/config/i386/i386-common.c (ix86_handle_option): Handle new options. * i386/i386.md (divsf3): Check OPTION_RECIP_DIV. (sqrt2): Check OPTION_RECIP_SQRT. * i386/sse.md (div3): Check OPTION_RECIP_VEC_DIV. (sqrt2): Check OPTION_RECIP_VEC_SQRT. * i386/i386.c (ix86_option_override_internal): Set recip_mask for -mrecip. (ix86_function_specific_save): Save recip_mask_explicit. (ix86_function_specific_restore): Restore recip_mask_explicit. * doc/invoke.texi (ix86 Options): Document the new options. Index: config/i386/i386.md === --- config/i386/i386.md (revision 178101) +++ config/i386/i386.md (working copy) @@ -7050,7 +7050,9 @@ (define_expand "divsf3" "(TARGET_80387 && X87_ENABLE_ARITH (SFmode)) || TARGET_SSE_MATH" { - if (TARGET_SSE_MATH && TARGET_RECIP && optimize_insn_for_speed_p () + if (TARGET_SSE_MATH + && OPTION_RECIP_DIV + && optimize_insn_for_speed_p () && flag_finite_math_only && !flag_trapping_math && flag_unsafe_math_optimizations) { @@ -13422,7 +13424,9 @@ (define_expand "sqrt2" || (SSE_FLOAT_MODE_P (mode) && TARGET_SSE_MATH)" { if (mode == SFmode - && TARGET_SSE_MATH && TARGET_RECIP && !optimize_function_for_size_p (cfun) + && TARGET_SSE_MATH + && OPTION_RECIP_SQRT + && !optimize_function_for_size_p (cfun) && flag_finite_math_only && !flag_trapping_math && flag_unsafe_math_optimizations) { Index: config/i386/sse.md === --- config/i386/sse.md (revision 178101) +++ config/i386/sse.md (working copy) @@ -772,7 +772,9 @@ (define_expand "div3" { ix86_fixup_binary_operands_no_copy (DIV, mode, operands); - if (TARGET_SSE_MATH && TARGET_RECIP && !optimize_insn_for_size_p () + if (TARGET_SSE_MATH + && OPTION_RECIP_VEC_DIV + && !optimize_insn_for_size_p () && flag_finite_math_only && !flag_trapping_math && flag_unsafe_math_optimizations) { @@ -850,7 +852,9 @@ (define_expand "sqrt2" (sqrt:VF1 (match_operand:VF1 1 "nonimmediate_operand" "")))] "TARGET_SSE" { - if (TARGET_SSE_MATH && TARGET_RECIP && !optimize_insn_for_size_p () + if (TARGET_SSE_MATH + && OPTION_RECIP_VEC_SQRT + && !optimize_insn_for_size_p () && flag_finite_math_only && !flag_trapping_math && flag_unsafe_math_optimizations) { Index: config/i386/i386.opt === --- config/i386/i386.opt(revision 178101) +++ config/i386/i386.opt(working copy) @@ -31,6 +31,9 @@ HOST_WIDE_INT ix86_isa_flags = TARGET_64 Variable HOST_WIDE_INT ix86_isa_flags_explicit +Variable +int recip_mask_explicit + ;; Definitions to add to the cl_target_option structure ;; -march= processor TargetSave @@ -56,6 +59,9 @@ HOST_WIDE_INT x_ix86_isa_flags_explicit TargetSave int ix86_target_flags_explicit +TargetSave +int x_recip_mask_explicit + ;; whether -mtune was not specified TargetSave unsigned char tune_defaulted @@ -373,6 +379,22 @@ mrecip Target Report Mask(RECIP) Save Generate reciprocals instead of divss and sqrtss. +mrecip-div +Target Mask(RECIP_DIV) Var(recip_mask) Save +Generate reciprocal estimations instead of scalar divisions. + +mrecip-sqrt +Target Mask(RECIP_SQRT) Var(recip_mask) Save +Generate reciprocal estimations instead of scalar sqrt. + +mrecip-vec-div +Target Mask(RECIP_VEC_DIV) Var(recip_mask) Save +Generate reciprocal estimations instead of vector divisions. + +mrecip-vec-sqrt +Target Mask(RECIP_VEC_SQRT) Var(recip_mask) Save +Generate reciprocal estimations instead of vector sqr
Re: [Ada] Implementation of aspects within generic units
On 31 Aug 2011, at 16:57, Arnaud Charlet wrote: (x86_64-unknown-linux-gnu) | | Storage_Error stack overflow or erroneous memory access | | Error detected at system.ads: 175:5 | + = = = = = = + Please include these source files with error report Note that list may not be accurate in some cases, so please double check that the problem can still be reproduced with the set of files listed. Consider also -gnatd.n switch (see debug.adb). /scratch/jmyers/fsf/gcc-mainline/gcc/ada/system.ads /scratch/jmyers/fsf/gcc-mainline/gcc/ada/a-charac.ads /scratch/jmyers/fsf/gcc-mainline/gcc/ada/ada.ads compilation abandoned make[3]: *** [ada/a-charac.o] Error 1 make[3]: *** Waiting for unfinished jobs [...] make[3]: Leaving directory `/scratch/jmyers/fsf/build/gcc' make[2]: *** [all-stage3-gcc] Error 2 make[2]: Leaving directory `/scratch/jmyers/fsf/build' make[1]: *** [stage3-bubble] Error 2 make[1]: Leaving directory `/scratch/jmyers/fsf/build' make: *** [all] Error 2 The bootstrap compiler is 4.6.2 20110816 (prerelease). OK. Can you get a backtrace from gnat1 on this crash so that we get a bit more info about where gnat1 is crashing? As I said, I can't reproduce it and the above info unfortunately isn't enough to understand what is going on. on i686-darwin9: (gdb) run Starting program: /Volumes/ScratchCS/gcc-4-7-trunk-build/prev-gcc/ gnat1 -I - -I . -I ada -I /GCC/gcc-live-trunk/gcc/ada -I /GCC/gcc-live- trunk/gcc/ada/gcc-interface -quiet -nostdinc -dumpbase a-charac.ads - auxbase-strip ada/a-charac.o -O2 -fexceptions -mmacosx-version- min=10.5.8 -g -gnatpg -gnata -gnatwns -mtune=core2 -fPIC -feliminate- unused-debug-symbols -gnatO ada/a-charac.o /GCC/gcc-live-trunk/gcc/ada/ a-charac.ads -o /var/folders/OW/OW-PGOtgHbKakssxFpJpkU++-0E/-Tmp-// ccKUPx8T.s Reading symbols for shared libraries +++.. done Program received signal EXC_BAD_ACCESS, Could not access memory. Reason: KERN_PROTECTION_FAILURE at address: 0x 0x001c4fa0 in lib__writ__write_ali () (gdb) bt #0 0x001c4fa0 in lib__writ__write_ali () #1 0x0036799a in _ada_gnat1drv () #2 0x000305f5 in gnat_parse_file () cheers, Iain
Re: [Ada] Implementation of aspects within generic units
On 31 Aug 2011, at 16:53, Joseph S. Myers wrote: On Wed, 31 Aug 2011, Arnaud Charlet wrote: +===GNAT BUG DETECTED==+ | 4.7.0 20110831 (experimental) [trunk revision 161655] (x86_64-unknown-linux-gnu) | | Storage_Error stack overflow or erroneous memory access | | Error detected at a-elchha.adb: 41:25 | This one was fixed very rapidly (like within 15 minutes AFAIK). It seems not. Still broken with rev. 178380. Hmm, then it's not the issue I had in mind, and not something I'm aware of (didn't get this error on my end on any build I've done). Which bootstrap compiler are you using, and at which stage is the above error occurring? Which options are you using to build? Did you restart a build from scratch after this error first occurred? I just retried another complete bootstrap from scratch and still don't get any error. I also see a bootstrap failure on x86_64-unknown-linux-gnu, r178381. +===GNAT BUG DETECTED======+ | 4.7.0 20110831 (experimental) [trunk revision 178381] (x86_64- unknown-linux-gnu) | | Storage_Error stack overflow or erroneous memory access | | Error detected at system.ads: 175:5 | | Please submit a bug report; see http://gcc.gnu.org/ bugs.html.| | Use a subject line meaningful to you and us to track the bug.| | Include the entire contents of this bug box in the report. | | Include the exact gcc or gnatmake command that you entered. | | Also include sources listed below in gnatchop format | | (concatenated together with no headers between files). | + = = = = = =+ Please include these source files with error report Note that list may not be accurate in some cases, so please double check that the problem can still be reproduced with the set of files listed. Consider also -gnatd.n switch (see debug.adb). /scratch/jmyers/fsf/gcc-mainline/gcc/ada/system.ads /scratch/jmyers/fsf/gcc-mainline/gcc/ada/a-charac.ads /scratch/jmyers/fsf/gcc-mainline/gcc/ada/ada.ads compilation abandoned make[3]: *** [ada/a-charac.o] Error 1 make[3]: *** Waiting for unfinished jobs [...] make[3]: Leaving directory `/scratch/jmyers/fsf/build/gcc' make[2]: *** [all-stage3-gcc] Error 2 make[2]: Leaving directory `/scratch/jmyers/fsf/build' make[1]: *** [stage3-bubble] Error 2 make[1]: Leaving directory `/scratch/jmyers/fsf/build' make: *** [all] Error 2 The bootstrap compiler is 4.6.2 20110816 (prerelease). same on i686-darwin9 @ 178381 (Bootstrap compiler = 4.7.0 r178116, last successful bootstrap 178261) -- Joseph S. Myers jos...@codesourcery.com
Re: [Ada] Implementation of aspects within generic units
> (x86_64-unknown-linux-gnu) | > | Storage_Error stack overflow or erroneous memory access | > | Error detected at system.ads:175:5 | > +==+ > > > > Please include these source files with error report > Note that list may not be accurate in some cases, > so please double check that the problem can still > be reproduced with the set of files listed. > Consider also -gnatd.n switch (see debug.adb). > > /scratch/jmyers/fsf/gcc-mainline/gcc/ada/system.ads > /scratch/jmyers/fsf/gcc-mainline/gcc/ada/a-charac.ads > /scratch/jmyers/fsf/gcc-mainline/gcc/ada/ada.ads > > compilation abandoned > make[3]: *** [ada/a-charac.o] Error 1 > make[3]: *** Waiting for unfinished jobs > [...] > make[3]: Leaving directory `/scratch/jmyers/fsf/build/gcc' > make[2]: *** [all-stage3-gcc] Error 2 > make[2]: Leaving directory `/scratch/jmyers/fsf/build' > make[1]: *** [stage3-bubble] Error 2 > make[1]: Leaving directory `/scratch/jmyers/fsf/build' > make: *** [all] Error 2 > > The bootstrap compiler is 4.6.2 20110816 (prerelease). OK. Can you get a backtrace from gnat1 on this crash so that we get a bit more info about where gnat1 is crashing? As I said, I can't reproduce it and the above info unfortunately isn't enough to understand what is going on. Arno
Re: [Ada] Implementation of aspects within generic units
On Wed, 31 Aug 2011, Arnaud Charlet wrote: > > >> +===GNAT BUG > > >> DETECTED======+ > > >> | 4.7.0 20110831 (experimental) [trunk revision 161655] > > >> (x86_64-unknown-linux-gnu) | > > >> | Storage_Error stack overflow or erroneous memory access > > >> | > > >> | Error detected at a-elchha.adb:41:25 > > >> | > > > > > > This one was fixed very rapidly (like within 15 minutes AFAIK). > > > > It seems not. Still broken with rev. 178380. > > Hmm, then it's not the issue I had in mind, and not something I'm aware of > (didn't get this error on my end on any build I've done). > > Which bootstrap compiler are you using, and at which stage is the above > error occurring? Which options are you using to build? > > Did you restart a build from scratch after this error first occurred? > > I just retried another complete bootstrap from scratch and still don't > get any error. I also see a bootstrap failure on x86_64-unknown-linux-gnu, r178381. +===GNAT BUG DETECTED==+ | 4.7.0 20110831 (experimental) [trunk revision 178381] (x86_64-unknown-linux-gnu) | | Storage_Error stack overflow or erroneous memory access | | Error detected at system.ads:175:5 | | Please submit a bug report; see http://gcc.gnu.org/bugs.html.| | Use a subject line meaningful to you and us to track the bug.| | Include the entire contents of this bug box in the report. | | Include the exact gcc or gnatmake command that you entered. | | Also include sources listed below in gnatchop format | | (concatenated together with no headers between files). | +==+ Please include these source files with error report Note that list may not be accurate in some cases, so please double check that the problem can still be reproduced with the set of files listed. Consider also -gnatd.n switch (see debug.adb). /scratch/jmyers/fsf/gcc-mainline/gcc/ada/system.ads /scratch/jmyers/fsf/gcc-mainline/gcc/ada/a-charac.ads /scratch/jmyers/fsf/gcc-mainline/gcc/ada/ada.ads compilation abandoned make[3]: *** [ada/a-charac.o] Error 1 make[3]: *** Waiting for unfinished jobs [...] make[3]: Leaving directory `/scratch/jmyers/fsf/build/gcc' make[2]: *** [all-stage3-gcc] Error 2 make[2]: Leaving directory `/scratch/jmyers/fsf/build' make[1]: *** [stage3-bubble] Error 2 make[1]: Leaving directory `/scratch/jmyers/fsf/build' make: *** [all] Error 2 The bootstrap compiler is 4.6.2 20110816 (prerelease). -- Joseph S. Myers jos...@codesourcery.com
Re: Vector shuffling
On Wed, 31 Aug 2011, Artem Shinkarov wrote: > 1) Helper function for the pseudo-builtins. > In my case the builtin can have 2 or 3 arguments, and I think that I > expressed that in a pretty much short way without any helper function. > Am I missing something? The point is to refactor what's common between this and other pseudo-builtins, not to have two pseudo-builtins doing things one way and one doing them another way -- Joseph S. Myers jos...@codesourcery.com
Re: [C++0x] contiguous bitfields race implementation
*bit_offset = (TREE_INT_CST_LOW (DECL_FIELD_OFFSET (fld)) * BITS_PER_UNIT + TREE_INT_CST_LOW (DECL_FIELD_BIT_OFFSET (fld))) - (TREE_INT_CST_LOW (DECL_FIELD_OFFSET (bitregion_start)) * BITS_PER_UNIT + TREE_INT_CST_LOW (DECL_FIELD_BIT_OFFSET (bitregion_start))); (Yes, I know we can factor out the BITS_PER_UNIT and only do one multiplication, it's just easier to read this way.) Is this what you had in mind? Yes. For convenience I'd simply use double_ints for the intermediate calculations. Ok, let's leave it like this for now. I have added a FIXME note, and we can optimize this after we get everything working.
Re: [Ada] Implementation of aspects within generic units
> >> +===GNAT BUG > >> DETECTED======+ > >> | 4.7.0 20110831 (experimental) [trunk revision 161655] > >> (x86_64-unknown-linux-gnu) | > >> | Storage_Error stack overflow or erroneous memory access > >> | > >> | Error detected at a-elchha.adb:41:25 > >> | > > > > This one was fixed very rapidly (like within 15 minutes AFAIK). > > It seems not. Still broken with rev. 178380. Hmm, then it's not the issue I had in mind, and not something I'm aware of (didn't get this error on my end on any build I've done). Which bootstrap compiler are you using, and at which stage is the above error occurring? Which options are you using to build? Did you restart a build from scratch after this error first occurred? I just retried another complete bootstrap from scratch and still don't get any error. Arno
Re: [Ada] Implementation of aspects within generic units
On Wed, Aug 31, 2011 at 1:16 PM, Arnaud Charlet wrote: >> Bootstrap with Ada included was broken at random revisions throughout >> the last days which made testing any patch quite painful (well, as someone >> who includes Ada in bootstrap and testing by default). >> >> Can you please ensure that you don't break bootstrap all the time? I'm >> now simply testing patches w/o Ada for the time being (having wasted >> another hour for verifying it wasn't my patch causing the last bootstrap >> error I ran into, loads of > > Sorry about the pain. Yes, I'm trying hard to not break bootstrap, and when > I do, I fix it as soon as possible. > >> +===GNAT BUG DETECTED==+ >> | 4.7.0 20110831 (experimental) [trunk revision 161655] >> (x86_64-unknown-linux-gnu) | >> | Storage_Error stack overflow or erroneous memory access | >> | Error detected at a-elchha.adb:41:25 | > > This one was fixed very rapidly (like within 15 minutes AFAIK). It seems not. Still broken with rev. 178380. Richard.
[PING] [PATCH] PR c++/47346 - access control for nested type is ignored in template
I am friendly pinging this patch. --- Begin Message --- Hello, Consider this code snippet: class C { struct Private { }; }; template struct exploit1 { C::Private type; }; exploit1 x1; //#1 At the instantiation point of exploit1 in #1, the compiler should issue an error because C::Private is private in that context. The infrastructure I have added a while back to do access checking of references to types like this was limited to typedefs only. This bug seems to suggest that the access checking of references to types, at template instantiation should be done for references to all types, not just typedefs. But then, consider this other example: class C { struct Private { }; }; template struct exploit2 : C::Private { }; exploit2 x2; g++ should warn here too, because access to C::Private is private in this context. The problem is when add_typedef_to_current_template_for_access_check is called (from check_accessibility_of_qualified_id), during the parsing of the class header, the "current template" representing exploit2 is not yet created. So the reference to C::Private cannot be added to that not-yet created template. The patch below loosens the existing typedefs access checking mechanisms to make it work for all type references, and defers the scheduling of that access checking when the relevant template is not yet created, until it becomes available. It also adjusts two test cases that were using similar invalid constructs that are now caught by the compiler. Tested on x86_64-unknown-linux-gnu against trunk. gcc/cp/ * cp-tree.h (struct qualified_type_usage_s): Renamed struct qualified_typedef_usage_s into this. (qualified_type_usage_t): Renamed qualified_typedef_usage_t into this. (struct tree_template_info::type_needing_access_checking): Renamed typedefs_needing_access_checking into this. (TI_TYPES_NEEDING_ACCESS_CHECKING): Renamed TI_TYPEDEFS_NEEDING_ACCESS_CHECKING into this. (add_type_decl_to_current_template_for_access_check): Renamed add_typedef_to_current_template_for_access_check into this. (cp_parsing_class_head, cp_no_type_access_check_p): Declare new public global variables. (defer_type_access_check_for_cur_template) (schedule_deferred_access_check_types_for_template): New functions. * semantics.c (add_type_decl_to_current_template_for_access_check): Renamed add_typedef_to_current_template_for_access_check into this. Make it accept potentially accesses to all types, not just typedefs. Make it defer the adding of the type access to the relevant template when said template is not yet available. (check_accessibility_of_qualified_id): Adjust. * decl.c (make_typename_type): Adjust. * parser.c (cp_parsing_class_head, cp_no_type_access_check_p): Define new public global variables. (cp_parser_elaborated_type_specifier): Don't schedule access checks for qualified type names of friend declaration, when in a template context. (cp_parser_class_specifier_1): Schedule qualified type names access check for type names access that happened while parsing the class head of a class template. (cp_parser_class_head): Notify the world when we are parsing a class head. * pt.c (cur_template_deferred_types_to_access_check): Define new global static variable. (perform_types_access_check): Renamed perform_typedefs_access_check into this and adjust the body. (instantiate_class_template_1, get_types_needing_access_check) (append_type_to_template_for_access_check_1) (append_type_to_template_for_access_check): Adjust. (defer_type_access_check_for_cur_template) (schedule_deferred_access_check_types_for_template): Define new functions. gcc/testsuite/ * g++.dg/template/access23.C: New test case. * g++.dg/lto/20081219_1.C: Adjust. * g++.dg/lto/20091002-1_0.C: Likewise. --- gcc/cp/cp-tree.h | 27 + gcc/cp/decl.c|4 +- gcc/cp/parser.c | 27 - gcc/cp/pt.c | 92 +++--- gcc/cp/semantics.c | 59 --- gcc/testsuite/g++.dg/lto/20081219_1.C|1 + gcc/testsuite/g++.dg/lto/20091002-1_0.C |1 + gcc/testsuite/g++.dg/template/access23.C | 33 +++ 8 files changed, 187 insertions(+), 57 deletions(-) create mode 100644 gcc/testsuite/g++.dg/template/access23.C diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h index ff5509e..3b01573 100644 --- a/gcc/cp/cp-tree.h +++ b/gcc/cp/cp-tree.h @@ -696,18 +696,18 @@ struct GTY (()) tree_lambda_expr In bar, the triplet will
[PATCH] Make VEC_COND_EXPR a GIMPLE_TERNARY_RHS
$subject says it all, COND_EXPR to follow. rhs1 will still be a "single" tree, namely a is_gimple_condexpr () operand. That's not easy to change (without forcing it to a separate statement, thus making it a is_gimple_val) because it embeds another tree code. Bootstrapped and tested on x86_64-unknown-linux-gnu. Any serious objections? Thanks, Richard. 2011-08-31 Richard Guenther * expr.c (expand_expr_real_2): Move VEC_COND_EXPR handling here, from ... (expand_expr_real_1): ... here. * gimple-pretty-print.c (dump_ternary_rhs): Handle VEC_COND_EXPR. * gimple.c (gimple_rhs_class_table): Make VEC_COND_EXPR a GIMPLE_TERNARY_RHS. * tree-cfg.c (verify_gimple_assign_ternary): Handle VEC_COND_EXPR here ... (verify_gimple_assign_single): ... not here. Index: trunk/gcc/expr.c === *** trunk.orig/gcc/expr.c 2011-08-29 16:57:31.0 +0200 --- trunk/gcc/expr.c2011-08-31 13:45:09.0 +0200 *** expand_expr_real_2 (sepops ops, rtx targ *** 8636,8641 --- 8636,8645 return temp; } + case VEC_COND_EXPR: + target = expand_vec_cond_expr (type, treeop0, treeop1, treeop2, target); + return target; + default: gcc_unreachable (); } *** expand_expr_real_1 (tree exp, rtx target *** 9932,9941 OK_DEFER_POP; return temp; - case VEC_COND_EXPR: - target = expand_vec_cond_expr (type, treeop0, treeop1, treeop2, target); - return target; - case MODIFY_EXPR: { tree lhs = treeop0; --- 9936,9941 Index: trunk/gcc/gimple-pretty-print.c === *** trunk.orig/gcc/gimple-pretty-print.c2011-07-11 17:02:51.0 +0200 --- trunk/gcc/gimple-pretty-print.c 2011-08-31 13:38:42.0 +0200 *** dump_ternary_rhs (pretty_printer *buffer *** 428,433 --- 428,443 pp_string (buffer, ">"); break; + case VEC_COND_EXPR: + pp_string (buffer, "VEC_COND_EXPR <"); + dump_generic_node (buffer, gimple_assign_rhs1 (gs), spc, flags, false); + pp_string (buffer, ", "); + dump_generic_node (buffer, gimple_assign_rhs2 (gs), spc, flags, false); + pp_string (buffer, ", "); + dump_generic_node (buffer, gimple_assign_rhs3 (gs), spc, flags, false); + pp_string (buffer, ">"); + break; + default: gcc_unreachable (); } Index: trunk/gcc/gimple.c === *** trunk.orig/gcc/gimple.c 2011-08-16 15:01:54.0 +0200 --- trunk/gcc/gimple.c 2011-08-31 13:27:41.0 +0200 *** get_gimple_rhs_num_ops (enum tree_code c *** 2615,2620 --- 2615,2621 || (SYM) == WIDEN_MULT_MINUS_EXPR \ || (SYM) == DOT_PROD_EXPR \ || (SYM) == REALIGN_LOAD_EXPR \ + || (SYM) == VEC_COND_EXPR \ || (SYM) == FMA_EXPR) ? GIMPLE_TERNARY_RHS \ : ((SYM) == COND_EXPR \ || (SYM) == CONSTRUCTOR \ *** get_gimple_rhs_num_ops (enum tree_code c *** 2622,2629 || (SYM) == ASSERT_EXPR \ || (SYM) == ADDR_EXPR \ || (SYM) == WITH_SIZE_EXPR \ ! || (SYM) == SSA_NAME\ ! || (SYM) == VEC_COND_EXPR) ? GIMPLE_SINGLE_RHS \ : GIMPLE_INVALID_RHS), #define END_OF_BASE_TREE_CODES (unsigned char) GIMPLE_INVALID_RHS, --- 2623,2629 || (SYM) == ASSERT_EXPR \ || (SYM) == ADDR_EXPR \ || (SYM) == WITH_SIZE_EXPR \ ! || (SYM) == SSA_NAME) ? GIMPLE_SINGLE_RHS \ : GIMPLE_INVALID_RHS), #define END_OF_BASE_TREE_CODES (unsigned char) GIMPLE_INVALID_RHS, Index: trunk/gcc/tree-cfg.c === *** trunk.orig/gcc/tree-cfg.c 2011-08-29 13:56:18.0 +0200 --- trunk/gcc/tree-cfg.c2011-08-31 13:42:01.0 +0200 *** verify_gimple_assign_ternary (gimple stm *** 3761,3767 return true; } ! if (!is_gimple_val (rhs1) || !is_gimple_val (rhs2) || !is_gimple_val (rhs3)) { --- 3761,3768 return true; } ! if
Re: [PATCH PR43513, 1/3] Replace vla with array - Implementation.
On Sat, Jul 30, 2011 at 12:21 AM, Tom de Vries wrote: > > This is an updated version of the patch. I have 2 new patches and an updated > testcase which I will sent out individually. > > Patch set was bootstrapped and reg-tested on x86_64. > > Ok for trunk? > > Thanks, > - Tom > > 2011-07-30 Tom de Vries > > PR middle-end/43513 > * Makefile.in (tree-ssa-ccp.o): Add $(PARAMS_H) to rule. > * tree-ssa-ccp.c (params.h): Include. > (fold_builtin_alloca_for_var): New function. > (ccp_fold_stmt): Use fold_builtin_alloca_for_var. > This caused: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50251 -- H.J.
PING PING Re: [Patch 0/4] ARM 64 bit sync atomic operations [V2]
On 9 August 2011 11:30, David Gilbert wrote: > Other than Ramana's comment about my comment on 4/4, has anyone else got any > other input? Otherwise I'd like to fix that comment and then get it in. We now have comments from Ramana on patches 3/4 and 4/4 - anyone for anything on the main pair ? Dave > On 26 July 2011 09:59, Dr. David Alan Gilbert > wrote: >> Hi, >> This is V2 of a series of 4 patches relating to ARM atomic operations; >> they incorporate most of the feedback from V1 - thanks Ramana, Richard and >> Joseph for comments. >> >> 1) Provide 64 bit atomic operations using the new ldrexd/strexd in ARMv6k >> and above. >> 2) Provide fallbacks so that when compiled for earlier CPUs a Linux kernel >> asssist is called (as per 32bit and smaller ops) >> 3) Fix pr48126 which is a misplaced barrier in the atomic generation >> 4) Correct the definition of TARGET_HAVE_DMB_MCR so that it doesn't >> produce the mcr instruction in Thumb1 (and enable on ARMv6 not just 6k >> as per the docs). >> >> Relative to v1: >> Split the DMB_MCR patch out >> Provide complete changelogs >> Don't emit IT instruction except in Thumb2 mode >> Move iterators to iterators.md (didn't move the table since it was specific >> to sync.md) >> Remove sync_atleastsi >> Use sync_predtab in as many places as possible >> Avoid headers in libgcc >> Made various libgcc routines I added static >> used __write instead of write >> Comment the barrier move to explain it more >> >> Note that the kernel interface has remained the same for the helper, and as >> such I've not changed the way the helper calling in patch 2 is structured. >> >> This code was tested with a full bootstrap on ARM; make check results >> are the same as without the patches except for extra passes due to the new >> tests. >> >> This work is part of Linaro blueprint: >> https://blueprints.launchpad.net/linaro-toolchain-misc/+spec/64-bit-sync-primitives >> >> Dave >> >> >
Re: [PATCH Atom][PR middle-end/44382] Tree reassociation improvement
Hello Richard, Thanks for the review! > The fact that you have to adjust gcc.dg/tree-ssa/pr38533.c looks problematic > to me. Can you investigate the problem report, look at the geneated > code with the atom default of the param and see whether it's still > reasonably "fixed" (maybe you'd already done this)? This test was created as a regression test to the problem in linearize_expr_tree which moves all statements down to the first modified one during reassociation increasing registers pressure. Test has a lot of definitions which are all ORed like this: def r1 def r2 s = r1 or r2 def r3 s = s or r3 def r4 s = s or r4 ... and it checks that register pressure is not increased after reassociation by simple scan of two sequential defs. If we use reassociation width higher than 1 then test will fails because we need to increase register pressure to get parallelism and finally get code like this: def r1 def r2 def r3 t1 = r1 or r2 s = s or r3 def r4 def r5 s = s or t1 t1 = r4 or r5 ... So, I had to fix a test. > > + /* Check if we may use less width and still compute sequence for > + the same time. It will allow us to reduce registers usage. */ > + while (width > 1 > + && get_required_cycles (ops_num, width - 1) == cycles_best) > + width--; > > I suppose get_required_cycles () is monotonic in width? Would it > make sense to binary search the best width then (I realize the > default is 1 and the only target differing has 2, but ...)? Maybe at > least add a comment to this effect? Or not decrement by one > but instead divide by two on each iteration (which is the same for 1 and 2 > ...)? > It's also all a mapping that is constant - we should be able to > pre-compute it for all values, eventually "compressing" it to a > much simpler formula width = f (cpu_width, ops_num)? I replaced sequential search with a binary search. I did not pay much attention to this function because do not think it is really time consuming compared to other parts of reassociation phase. Am I too optimistic? If you think it might significantly affect compile time I can introduce a table of pre-computed values (or make it later as a separate fix). I made all other fixes as you suggested. Bootstrapped and checked on x86_64-linux. Thanks, Ilya --- gcc/ 2011-08-31 Enkovich Ilya PR middle-end/44382 * target.def (reassociation_width): New hook. * doc/tm.texi.in (reassociation_width): Likewise. * doc/tm.texi (reassociation_width): Likewise. * doc/invoke.texi (tree-reassoc-width): New param documented. * hooks.h (hook_int_uint_mode_1): New default hook. * hooks.c (hook_int_uint_mode_1): Likewise. * config/i386/i386.h (ix86_tune_indices): Add X86_TUNE_REASSOC_INT_TO_PARALLEL and X86_TUNE_REASSOC_FP_TO_PARALLEL. (TARGET_REASSOC_INT_TO_PARALLEL): New. (TARGET_REASSOC_FP_TO_PARALLEL): Likewise. * config/i386/i386.c (initial_ix86_tune_features): Add X86_TUNE_REASSOC_INT_TO_PARALLEL and X86_TUNE_REASSOC_FP_TO_PARALLEL. (ix86_reassociation_width) implementation of new hook for i386 target. * params.def (PARAM_TREE_REASSOC_WIDTH): New param added. * tree-ssa-reassoc.c (get_required_cycles): New function. (get_reassociation_width): Likewise. (swap_ops_for_binary_stmt): Likewise. (rewrite_expr_tree_parallel): Likewise. (rewrite_expr_tree): Refactored. Part of code moved into swap_ops_for_binary_stmt. (reassociate_bb): Now checks reassociation width to be used and call rewrite_expr_tree_parallel instead of rewrite_expr_tree if needed. gcc/testsuite/ 2011-08-31 Enkovich Ilya * gcc.dg/tree-ssa/pr38533.c (dg-options): Added option --param tree-reassoc-width=1. * gcc.dg/tree-ssa/reassoc-24.c: New test. * gcc.dg/tree-ssa/reassoc-25.c: Likewise. PR44382.diff Description: Binary data
Re: Vector shuffling
On Wed, Aug 31, 2011 at 1:02 PM, Artem Shinkarov wrote: > Here is a newer version of the patch, which transforms the builtin to > the VEC_SHUFFLE_EXPR in the front-end. > > Several comments: > 1) Helper function for the pseudo-builtins. > In my case the builtin can have 2 or 3 arguments, and I think that I > expressed that in a pretty much short way without any helper function. > Am I missing something? > > 2) Richard, why do you want to treat VEC_SHUFFLE_EXPR as > GIMPLE_TERNARY_RHS when VEC_COND_EXPR (which is about the same as > VEC_SHUFF_EXPR) is single_rhs? From my perspective I don't see much of > a difference whether it is trenary or single, so I converted it to > trenary as you asked. But still it looks suspicious that vec_cond and > vec_shuffle are treated differently. VEC_SHUFFLE_EXPR has three operands, VEC_COND_EXPR is a little weird, as it has a sub-expression - if you count that as a single operand then it would have three too. I'll consider converting it to ternary, too. Richard. > Can anyone review the x86 parts? > > > Thanks, > Artem. >
Re: [Ada] Implementation of aspects within generic units
> Bootstrap with Ada included was broken at random revisions throughout > the last days which made testing any patch quite painful (well, as someone > who includes Ada in bootstrap and testing by default). > > Can you please ensure that you don't break bootstrap all the time? I'm > now simply testing patches w/o Ada for the time being (having wasted > another hour for verifying it wasn't my patch causing the last bootstrap > error I ran into, loads of Sorry about the pain. Yes, I'm trying hard to not break bootstrap, and when I do, I fix it as soon as possible. > +===GNAT BUG DETECTED======+ > | 4.7.0 20110831 (experimental) [trunk revision 161655] > (x86_64-unknown-linux-gnu) | > | Storage_Error stack overflow or erroneous memory access | > | Error detected at a-elchha.adb:41:25 | This one was fixed very rapidly (like within 15 minutes AFAIK). The other issue reported was wrt rts vs $(RTS) which wasn't detected by my build and which was fixed later, so perhaps it's the one that caused you more troubles. In any case, I'll try to avoid these as much as possible. Arno
Re: [C++0x] contiguous bitfields race implementation
On Wed, Aug 31, 2011 at 9:45 AM, Richard Guenther wrote: > On Tue, Aug 30, 2011 at 5:01 PM, Aldy Hernandez wrote: >> [I'm going to respond to this piece-meal, to make sure I don't drop >> anything. My apologies for the long thread, but I'm pretty sure it's in >> everybody's kill file by now.] >> >>> + /* Be as conservative as possible on variable offsets. */ >>> + if (TREE_OPERAND (exp, 2) >>> +&& !host_integerp (TREE_OPERAND (exp, 2), 1)) >>> + { >>> + *byte_offset = TREE_OPERAND (exp, 2); >>> + *maxbits = BITS_PER_UNIT; >>> + return; >>> + } >>> >>> shouldn't this be at the very beginning of the function? Because >>> you've set *bit_offset to an offset that was _not_ calculated relative >> >> Sure. I assume in this case, *bit_offset would be 0, right? > > It would be DECL_FIELD_BIT_OFFSET of that field. Oh, and > *byte_offset would be > > *byte_offset = size_binop (MULT_EXPR, TREE_OPERAND (exp, 2), > size_int (DECL_OFFSET_ALIGN > (field) / BITS_PER_UNIT)); > > see expr.c:component_ref_field_offset () (which you conveniently > could use here). > > Note that both TREE_OPERAND (exp, 2) and compoment_ref_field_offset > return offsets relative to the immediate containing struct type, not > relative to the base object like get_inner_reference does ... > (where it is still unclear to me what we are supposed to return from this > function ...) > > Thus, conservative would be using get_inner_reference here, if the > offset is supposed to be relative to the base object. That said, shouldn't *maxbits not at least make sure to cover the field itself? > Richard. >
[v3] Implement LWG 2004
Hi, tested x86_64-linux, committed to mainline. Paolo. /// 2011-08-31 Paolo Carlini * include/std/chrono (operator*(const _Rep1&, const duration<>&)): Fix order of template parameters per LWG 2004. Index: include/std/chrono === --- include/std/chrono (revision 178363) +++ include/std/chrono (working copy) @@ -395,7 +395,7 @@ return __cd(__cd(__d).count() * __s); } -template +template constexpr duration::type, _Period> operator*(const _Rep1& __s, const duration<_Rep2, _Period>& __d)
Re: [Ada] Implementation of aspects within generic units
On Wed, Aug 31, 2011 at 11:43 AM, Arnaud Charlet wrote: > If a declaration within a generic unit has aspects, the capture of references > within the aspect expressions has to be done in a separate step because the > aspect specificatios are not directly attached to the tree for the > declaration. Just replying to a random mail of this mega-series. Bootstrap with Ada included was broken at random revisions throughout the last days which made testing any patch quite painful (well, as someone who includes Ada in bootstrap and testing by default). Can you please ensure that you don't break bootstrap all the time? I'm now simply testing patches w/o Ada for the time being (having wasted another hour for verifying it wasn't my patch causing the last bootstrap error I ran into, loads of +===GNAT BUG DETECTED==+ | 4.7.0 20110831 (experimental) [trunk revision 161655] (x86_64-unknown-linux-gnu) | | Storage_Error stack overflow or erroneous memory access | | Error detected at a-elchha.adb:41:25 | Thanks, Richard. > The following must compile quietly: > > gcc -c -gnat12 -gnata ml.ads > > with Ml_Type; > with Generic_Matrix; > package Ml is > > subtype T_Int32 is Ml_Type.T_Int32; > subtype T_Float32 is Ml_Type.T_Float32; > > package M32 is new Generic_Matrix(G_Float => T_Float32); > type T_Matrix32 is new M32.G_Matrix; > > subtype Range2 is T_Int32 range 0 .. 1; > subtype Range3 is T_Int32 range 0 .. 2; > end Ml; > --- > package Ml_Type is > type T_Int32 is range (-2 ** 31) .. (2 ** 31 - 1); > > type T_Float32 is digits 6 range -3.40282E+38 .. 3.40282E+38; > end Ml_Type; > --- > with Ml_Type; > > generic > type G_Float is digits <>; > package Generic_Matrix is > type G_Matrix is array (Ml_Type.T_Int32 range <>, > Ml_Type.T_Int32 range <>) of G_Float; > > function "+" (Left, > Right : G_Matrix) > return G_Matrix; > > function "-" (Left, > Right : G_Matrix) > return G_Matrix > with Pre => (Left'Length (1) = Right'Length (1) and then > Left'Length (2) = Right'Length (2) ); > end Generic_Matrix; > --- > package body Generic_Matrix is > function "+" (Left, > Right : G_Matrix) > return G_Matrix is > Res : G_Matrix(Left'Range(1), Left'Range(2)); > begin > > if Left'Length (1) /= Right'Length (1) > or else Left'Length (2) /= Right'Length (2) > then > raise Constraint_Error with > "matrices are of different dimension in elementwise operation"; > end if; > > for I in Res'Range(1) loop > for J in Res'Range(2) loop > Res(I,J) := Left(I,J) + Right(I,J); > end loop; > end loop; > > return Res; > end "+"; > > function "-" (Left, > Right : G_Matrix) > return G_Matrix is > Res : G_Matrix(Left'Range(1), Left'Range(2)); > begin > > for I in Res'Range(1) loop > for J in Res'Range(2) loop > Res(I,J) := Left(I,J) - Right(I,J); > end loop; > end loop; > > return Res; > end "-"; > end Generic_Matrix; > > Tested on x86_64-pc-linux-gnu, committed on trunk > > 2011-08-31 Ed Schonberg > > * sem_ch12.adb (Save_References): If the node has aspects, save > references within the corresponding expressions in a separate step, > because the aspects are not directly in the tree for the declaration > to which they belong. > >
[Ada] Implementation of aspects within generic units
If a declaration within a generic unit has aspects, the capture of references within the aspect expressions has to be done in a separate step because the aspect specificatios are not directly attached to the tree for the declaration. The following must compile quietly: gcc -c -gnat12 -gnata ml.ads with Ml_Type; with Generic_Matrix; package Ml is subtype T_Int32 is Ml_Type.T_Int32; subtype T_Float32 is Ml_Type.T_Float32; package M32 is new Generic_Matrix(G_Float => T_Float32); type T_Matrix32 is new M32.G_Matrix; subtype Range2 is T_Int32 range 0 .. 1; subtype Range3 is T_Int32 range 0 .. 2; end Ml; --- package Ml_Type is type T_Int32 is range (-2 ** 31) .. (2 ** 31 - 1); type T_Float32 is digits 6 range -3.40282E+38 .. 3.40282E+38; end Ml_Type; --- with Ml_Type; generic type G_Float is digits <>; package Generic_Matrix is type G_Matrix is array (Ml_Type.T_Int32 range <>, Ml_Type.T_Int32 range <>) of G_Float; function "+" (Left, Right : G_Matrix) return G_Matrix; function "-" (Left, Right : G_Matrix) return G_Matrix with Pre => (Left'Length (1) = Right'Length (1) and then Left'Length (2) = Right'Length (2) ); end Generic_Matrix; --- package body Generic_Matrix is function "+" (Left, Right : G_Matrix) return G_Matrix is Res : G_Matrix(Left'Range(1), Left'Range(2)); begin if Left'Length (1) /= Right'Length (1) or else Left'Length (2) /= Right'Length (2) then raise Constraint_Error with "matrices are of different dimension in elementwise operation"; end if; for I in Res'Range(1) loop for J in Res'Range(2) loop Res(I,J) := Left(I,J) + Right(I,J); end loop; end loop; return Res; end "+"; function "-" (Left, Right : G_Matrix) return G_Matrix is Res : G_Matrix(Left'Range(1), Left'Range(2)); begin for I in Res'Range(1) loop for J in Res'Range(2) loop Res(I,J) := Left(I,J) - Right(I,J); end loop; end loop; return Res; end "-"; end Generic_Matrix; Tested on x86_64-pc-linux-gnu, committed on trunk 2011-08-31 Ed Schonberg * sem_ch12.adb (Save_References): If the node has aspects, save references within the corresponding expressions in a separate step, because the aspects are not directly in the tree for the declaration to which they belong. Index: sem_ch12.adb === --- sem_ch12.adb(revision 178372) +++ sem_ch12.adb(working copy) @@ -12737,6 +12737,23 @@ end if; end; end if; + + -- If a node has aspects, references within their expressions must + -- be saved separately, given that they are not directly in the + -- tree. + + if Has_Aspects (N) then +declare + Aspect : Node_Id; + +begin + Aspect := First (Aspect_Specifications (N)); + while Present (Aspect) loop + Save_Global_References (Expression (Aspect)); + Next (Aspect); + end loop; +end; + end if; end Save_References; -- Start of processing for Save_Global_References
[Ada] Passing Stream_Element_Offset/Storage_Offset in heterogeneous DSA apps
This change ensures that partitions in a DSA application always use a 64 bit representation when exchanging data of type Stream_Element_Offset or Storage_Offset, to avoid interoperability issues between 32- and 64-bit hosts. Tested on x86_64-pc-linux-gnu, committed on trunk 2011-08-31 Thomas Quinot * rtsfind.ads, exp_dist.adb (Find_Numeric_Representation): Predefined types Stream_Element_Offset and Storage_Offset have a different native type depending on whether the platform is 32 or 64 bits. When exchanging them, always convert to 64 bits. Index: rtsfind.ads === --- rtsfind.ads (revision 178371) +++ rtsfind.ads (working copy) @@ -555,6 +555,7 @@ RE_Root_Stream_Type,-- Ada.Streams RE_Stream_Element, -- Ada.Streams + RE_Stream_Element_Offset, -- Ada.Streams RE_Stream_Access, -- Ada.Streams.Stream_IO @@ -1748,6 +1749,7 @@ RE_Root_Stream_Type => Ada_Streams, RE_Stream_Element => Ada_Streams, + RE_Stream_Element_Offset=> Ada_Streams, RE_Stream_Access=> Ada_Streams_Stream_IO, Index: exp_dist.adb === --- exp_dist.adb(revision 178358) +++ exp_dist.adb(working copy) @@ -10842,6 +10842,15 @@ P_Size : constant Uint := Esize (FST); begin +-- Special case: for Stream_Element_Offset and Storage_Offset, +-- always force transmission as a 64-bit value. + +if Is_RTE (FST, RE_Stream_Element_Offset) + or else Is_RTE (FST, RE_Storage_Offset) +then + return RTE (RE_Unsigned_64); +end if; + if Is_Unsigned_Type (Typ) then if P_Size <= 8 then return RTE (RE_Unsigned_8);
[Ada] Use Dir_Seps everywhere to properly handle all directory speparators.
On Windows the directory separator can be / or \, in some cases this was not properly handled. This patch fixes this issue. Tested on x86_64-pc-linux-gnu, committed on trunk 2011-08-31 Pascal Obry * a-direct.adb: Use Dir_Seps everywhere to properly handle all directory speparators. (Compose): Use Dir_Seps to handle both forms. (Create_Path): Use Dir_Seps instead of explicit check, no semantic changes. (Extension): Use Dir_Seps to handle both forms. Index: a-direct.adb === --- a-direct.adb(revision 178358) +++ a-direct.adb(working copy) @@ -32,7 +32,7 @@ with Ada.Calendar; use Ada.Calendar; with Ada.Calendar.Formatting;use Ada.Calendar.Formatting; with Ada.Directories.Validity; use Ada.Directories.Validity; -with Ada.Strings.Maps; +with Ada.Strings.Maps; use Ada; use Ada.Strings.Maps; with Ada.Strings.Fixed; with Ada.Strings.Unbounded; use Ada.Strings.Unbounded; with Ada.Unchecked_Conversion; @@ -61,8 +61,7 @@ pragma Import (C, Dir_Separator, "__gnat_dir_separator"); -- Running system default directory separator - Dir_Seps : constant Ada.Strings.Maps.Character_Set := -Ada.Strings.Maps.To_Set ("/\"); + Dir_Seps : constant Character_Set := Strings.Maps.To_Set ("/\"); -- UNIX and DOS style directory separators Max_Path : Integer; @@ -175,7 +174,7 @@ -- Add a directory separator if needed - if Last /= 0 and then Result (Last) /= Dir_Separator then + if Last /= 0 and then not Is_In (Result (Last), Dir_Seps) then Last := Last + 1; Result (Last) := Dir_Separator; end if; @@ -457,17 +456,13 @@ -- Look for the end of an intermediate directory -if New_Dir (J) /= Dir_Separator and then - New_Dir (J) /= '/' -then +if not Is_In (New_Dir (J), Dir_Seps) then Last := J; -- We have found a new intermediate directory each time we find -- a first directory separator. -elsif New_Dir (J - 1) /= Dir_Separator and then - New_Dir (J - 1) /= '/' -then +elsif not Is_In (New_Dir (J - 1), Dir_Seps) then -- No need to create the directory if it already exists @@ -664,7 +659,7 @@ -- If a directory separator is found before a dot, there is no -- extension. -if Name (Pos) = Dir_Separator then +if Is_In (Name (Pos), Dir_Seps) then return Empty_String; elsif Name (Pos) = '.' then
[Ada] Expansion of indexed referenced to packed arrays when prefix is a call
When an indexed reference to a packed array is expanded, it is rewritten as a boolean operation on the prefix that denotes the packed array. If the prefix is a function call it must not be reanalyzed, because it will have been expanded already. and may carry extra actuals for accessibility checks. No simple example available. Tested on x86_64-pc-linux-gnu, committed on trunk 2011-08-31 Ed Schonberg * exp_pakd.adb (Convert_To_PAT_Type): If prefix is a function call, do not reanalyze it. Index: exp_pakd.adb === --- exp_pakd.adb(revision 178358) +++ exp_pakd.adb(working copy) @@ -6,7 +6,7 @@ -- -- -- B o d y -- -- -- --- Copyright (C) 1992-2010, Free Software Foundation, Inc. -- +-- Copyright (C) 1992-2011, Free Software Foundation, Inc. -- -- -- -- GNAT is free software; you can redistribute it and/or modify it under -- -- terms of the GNU General Public License as published by the Free Soft- -- @@ -702,7 +702,9 @@ -- see Reset_Packed_Prefix. On the other hand, if the prefix is a simple -- array reference, reanalysis can produce spurious type errors when the -- PAT type is replaced again with the original type of the array. Same - -- for the case of a dereference. The following is correct and minimal, + -- for the case of a dereference. Ditto for function calls: expansion + -- may introduce additional actuals which will trigger errors if call + -- is reanalyzed. The following is correct and minimal, -- but the handling of more complex packed expressions in actuals is -- confused. Probably the problem only remains for actuals in calls. @@ -713,6 +715,7 @@ (Nkind (Aexp) = N_Indexed_Component and then Is_Entity_Name (Prefix (Aexp))) or else Nkind (Aexp) = N_Explicit_Dereference +or else Nkind (Aexp) = N_Function_Call then Set_Analyzed (Aexp); end if;
[Ada] Spurious errors on aggregates in instances
In an instance, errors that report that an aggregate (sub)component is limited must be ignaored because they originate in view conflicts. If the original aggregate is legal and the actuals are legal, the aggregate in the instance itself is legal. No simple example available. Tested on x86_64-pc-linux-gnu, committed on trunk 2011-08-31 Ed Schonberg * sem_aggr.adb (Resolve_Aggregate): In an instance, ignore aggregate subcomponents tnat may be limited, because they originate in view conflicts. If the original aggregate is legal and the actuals are legal, the aggregate itself is legal. Index: sem_aggr.adb === --- sem_aggr.adb(revision 178358) +++ sem_aggr.adb(working copy) @@ -1052,8 +1052,14 @@ end if; -- Ada 2005 (AI-287): Limited aggregates allowed + -- In an instance, ignore aggregate subcomponents tnat may be limited, + -- because they originate in view conflicts. If the original aggregate + -- is legal and the actuals are legal, the aggregate itself is legal. - if Is_Limited_Type (Typ) and then Ada_Version < Ada_2005 then + if Is_Limited_Type (Typ) +and then Ada_Version < Ada_2005 +and then not In_Instance + then Error_Msg_N ("aggregate type cannot be limited", N); Explain_Limited_Type (Typ, N);
[Ada] Fix spurious "prefix of dereference must be an access type"
This patch fixes an obscure bug, in which an error "prefix of dereference must be an access type" is given incorrectly. The spurious error would occur in an instance of a generic if an implicit dereference of an expression of a private type is used. No simple test is available. Tested on x86_64-pc-linux-gnu, committed on trunk 2011-08-31 Bob Duff * exp_ch4.adb (Expand_N_Selected_Component): Use the full type, in case the access type is private; we don't care about privacy in expansion. Index: exp_ch4.adb === --- exp_ch4.adb (revision 178368) +++ exp_ch4.adb (working copy) @@ -7920,6 +7920,7 @@ -- Insert explicit dereference if required if Is_Access_Type (Ptyp) then + Set_Etype (P, Ptyp); -- in case it's private Insert_Explicit_Dereference (P); Analyze_And_Resolve (P, Designated_Type (Ptyp));
Re: [DOC] CP_TYPE_QUALS -> cp_type_quals
.. I applied the patch after running make info and make dvi, seems obvious to me. Thanks, Paolo.
[Ada] Support for Priority aspect
Priority may be specified for protected and task types and objects using the standard aspect notation. This patch makes the analysis of the aspect argument earlier, so it is taken into account when the task or protected object is been created. The following must compile quietly in Ada2012 mode and execute without producing any output. with Priority_Aspect; procedure Test_Priority_Aspect is begin null; end Test_Priority_Aspect; with System; package Priority_Aspect is Prio : constant System.Priority := System.Priority'First; task type Aspect_Type with Priority => Prio; T : Aspect_Type; task Aspect_Task with Priority => Prio; protected type Pr_Type with Priority => Prio is procedure Proc; end Pr_Type; P : Pr_Type; protected Pr_Obj with Priority => Prio is procedure Proc; end Pr_Obj; end Priority_Aspect; with Ada.Text_IO; use Ada.Text_IO; with Ada.Dynamic_Priorities; use Ada.Dynamic_Priorities; package body Priority_Aspect is protected body Pr_Type is procedure Proc is begin if Pr_Type'Priority /= Prio then Put_Line ("Unexpected priority when using aspect in protected type"); end if; end Proc; end Pr_Type; protected body Pr_Obj is procedure Proc is begin if Pr_Obj'Priority /= Prio then Put_Line ("Unexpected priority when using aspect in protected object"); end if; end Proc; end Pr_Obj; task body Aspect_Type is begin if Get_Priority /= Prio then Put_Line ("Unexpected priority when using aspect in task type"); end if; P.Proc; end Aspect_Type; task body Aspect_Task is begin if Get_Priority /= Prio then Put_Line ("Unexpected priority when using aspect in task object"); end if; Pr_Obj.Proc; end Aspect_Task; end Priority_Aspect; Command: gnatmake -q -gnat12 test_priority_aspect; test_priority_aspect Tested on x86_64-pc-linux-gnu, committed on trunk 2011-08-31 Jose Ruiz * sem_ch13.adb (Analyze_Aspect_Specifications): For the Priority and Interrupt_Priority aspects, force the analysis of the aspect expression (when building the equivalent pragma). Otherwise, its analysis is done too late, after the task or protected object has been created. * sem_ch9.adb (Analyze_Single_Protected_Declaration, Analyze_Single_Task_Declaration): Remove the code to move the aspects to the object declaration because they are needed in the type declaration. Index: sem_ch9.adb === --- sem_ch9.adb (revision 178358) +++ sem_ch9.adb (working copy) @@ -23,7 +23,6 @@ -- -- -- -with Aspects; use Aspects; with Atree;use Atree; with Checks; use Checks; with Einfo;use Einfo; @@ -1726,7 +1725,6 @@ Defining_Identifier => O_Name, Object_Definition => Make_Identifier (Loc, Chars (T))); - Move_Aspects (N, O_Decl); Rewrite (N, T_Decl); Insert_After (N, O_Decl); Mark_Rewrite_Insertion (O_Decl); @@ -1796,7 +1794,6 @@ Defining_Identifier => O_Name, Object_Definition => Make_Identifier (Loc, Chars (T))); - Move_Aspects (N, O_Decl); Rewrite (N, T_Decl); Insert_After (N, O_Decl); Mark_Rewrite_Insertion (O_Decl); Index: sem_ch13.adb === --- sem_ch13.adb(revision 178358) +++ sem_ch13.adb(working copy) @@ -1164,7 +1164,9 @@ Pragma_Identifier=> Make_Identifier (Sloc (Id), Pname), Pragma_Argument_Associations => -New_List (Relocate_Node (Expr))); +New_List + (Make_Pragma_Argument_Association +(Sloc (Id), Expression => Relocate_Node (Expr; Set_From_Aspect_Specification (Aitem, True); @@ -1526,6 +1528,12 @@ end if; Prepend (Aitem, To => L); + + -- Analyze rewritten pragma. Otherwise, its + -- analysis is done too late, after the task or + -- protected object has been created. + + Analyze (Aitem); end; -- For all other cases, insert in sequence
[Ada] Use assertion to verify representation invariant for lock status
Replaced a comment about lock status with a proper assertion, and added a comment to explain the representation invariant for the lock status. Tested on x86_64-pc-linux-gnu, committed on trunk 2011-08-31 Matthew Heaney * a-rbtgbo.adb (Clear_Tree): Assert representation invariant for lock status. Index: a-rbtgbo.adb === --- a-rbtgbo.adb(revision 178368) +++ a-rbtgbo.adb(working copy) @@ -59,15 +59,16 @@ "attempt to tamper with cursors (container is busy)"; end if; + -- The lock status (which monitors "element tampering") always implies + -- that the busy status (which monitors "cursor tampering") is set too; + -- this is a representation invariant. Thus if the busy bit is not set, + -- then the lock bit must not be set either. + pragma Assert (Tree.Lock = 0); + Tree.First := 0; Tree.Last := 0; Tree.Root := 0; Tree.Length := 0; - - -- Why are the following commented out with no explanation ??? - -- Tree.Busy - -- Tree.Lock - Tree.Free := -1; end Clear_Tree;
[Ada] Delete SCIL files in CodePeer mode
When generating scil files (-gnatcC), we want to remove any old scil files corresponding to the main unit being compiled in case of errors, to avoid re-analyzing these old scil files. Tested on x86_64-pc-linux-gnu, committed on trunk 2011-08-31 Arnaud Charlet * comperr.adb, comperr.ads, gnat1drv.adb (Delete_SCIL_Files): New subprogram. (Compiler_Abort, Gnat1drv): Call Delete_SCIL_Files in codepeer mode in case of a compilation error. Index: comperr.adb === --- comperr.adb (revision 178358) +++ comperr.adb (working copy) @@ -6,7 +6,7 @@ -- -- -- B o d y -- -- -- --- Copyright (C) 1992-2009, Free Software Foundation, Inc. -- +-- Copyright (C) 1992-2011, Free Software Foundation, Inc. -- -- -- -- GNAT is free software; you can redistribute it and/or modify it under -- -- terms of the GNU General Public License as published by the Free Soft- -- @@ -27,20 +27,23 @@ -- error is detected. Calls to these routines cause termination of the -- current compilation with appropriate error output. -with Atree;use Atree; -with Debug;use Debug; -with Errout; use Errout; -with Gnatvsn; use Gnatvsn; -with Namet;use Namet; -with Opt; use Opt; -with Osint;use Osint; -with Output; use Output; -with Sinput; use Sinput; -with Sprint; use Sprint; -with Sdefault; use Sdefault; -with Targparm; use Targparm; -with Treepr; use Treepr; -with Types;use Types; +with Atree; use Atree; +with Debug; use Debug; +with Errout;use Errout; +with Gnatvsn; use Gnatvsn; +with Lib; use Lib; +with Namet; use Namet; +with Opt; use Opt; +with Osint; use Osint; +with Output;use Output; +with Sinfo; use Sinfo; +with Sinput;use Sinput; +with Sprint;use Sprint; +with Sdefault; use Sdefault; +with System.OS_Lib; use System.OS_Lib; +with Targparm; use Targparm; +with Treepr;use Treepr; +with Types; use Types; with Ada.Exceptions; use Ada.Exceptions; @@ -144,6 +147,10 @@ end if; end if; + if CodePeer_Mode then + Delete_SCIL_Files; + end if; + -- If any errors have already occurred, then we guess that the abort -- may well be caused by previous errors, and we don't make too much -- fuss about it, since we want to let programmer fix the errors first. @@ -422,9 +429,40 @@ Source_Dump; raise Unrecoverable_Error; end if; - end Compiler_Abort; + --- + -- Delete_SCIL_Files -- + --- + + procedure Delete_SCIL_Files is + Main: Node_Id; + Success : Boolean; + pragma Unreferenced (Success); + begin + -- If parsing was not successful, no Main_Unit is available, so return + -- immediately. + + if Main_Source_File = No_Source_File then + return; + end if; + + -- Retrieve unit name, and remove old versions of SCIL/.scil and + -- SCIL/__body.scil + + Main := Unit (Cunit (Main_Unit)); + + if Nkind (Main) = N_Subprogram_Body then + Get_Name_String (Chars (Defining_Unit_Name (Specification (Main; + else + Get_Name_String (Chars (Defining_Unit_Name (Main))); + end if; + + Delete_File ("SCIL/" & Name_Buffer (1 .. Name_Len) & ".scil", Success); + Delete_File +("SCIL/" & Name_Buffer (1 .. Name_Len) & "__body.scil", Success); + end Delete_SCIL_Files; + - -- Repeat_Char -- - Index: comperr.ads === --- comperr.ads (revision 178358) +++ comperr.ads (working copy) @@ -6,7 +6,7 @@ -- -- -- S p e c -- -- -- --- Copyright (C) 1992-2007, Free Software Foundation, Inc. -- +-- Copyright (C) 1992-2011, Free Software Foundation, Inc. -- -- -- -- GNAT is free software; you can redistribute it and/or modify it under -- -- terms of the GNU General Public License as published by the Free Soft- -- @@ -50,6 +50,9 @@ -- end exception (with possible message stored in TSD.Current_Excep, -- and negative (an unused value) for a GCC abort. + procedure Delete_SCIL_Files; + -- Delete SCIL files associated with t
[Ada] Detect all derived types as violation of the SPARK restriction
When the SPARK restriction was set, GNAT was not issuing violations on some derived types. Now corrected. Tested on x86_64-pc-linux-gnu, committed on trunk 2011-08-31 Marc Sango * restrict.adb (Check_SPARK_Restriction): Change Comes_From_Source (N) by Comes_From_Source (Original_Node (N)) in order to treat also the nodes which have been rewritten. * sem_ch4.adb (Analyze_Explicit_Dereference, Analyze_Slice): Guard the explicit dereference and slice violation in spark mode on the nodes coming only from the source code. Index: sem_ch4.adb === --- sem_ch4.adb (revision 178363) +++ sem_ch4.adb (working copy) @@ -1763,7 +1763,9 @@ -- Start of processing for Analyze_Explicit_Dereference begin - Check_SPARK_Restriction ("explicit dereference is not allowed", N); + if Comes_From_Source (N) then + Check_SPARK_Restriction ("explicit dereference is not allowed", N); + end if; -- In formal verification mode, keep track of all reads and writes -- through explicit dereferences. @@ -4417,7 +4419,9 @@ -- Start of processing for Analyze_Slice begin - Check_SPARK_Restriction ("slice is not allowed", N); + if Comes_From_Source (N) then + Check_SPARK_Restriction ("slice is not allowed", N); + end if; Analyze (P); Analyze (D); Index: restrict.adb === --- restrict.adb(revision 178358) +++ restrict.adb(working copy) @@ -117,7 +117,7 @@ Msg_Issued : Boolean; Save_Error_Msg_Sloc : Source_Ptr; begin - if Force or else Comes_From_Source (N) then + if Force or else Comes_From_Source (Original_Node (N)) then if Restriction_Check_Required (SPARK) and then Is_In_Hidden_Part_In_SPARK (Sloc (N)) @@ -145,7 +145,7 @@ begin pragma Assert (Msg2'Length /= 0 and then Msg2 (Msg2'First) = '\'); - if Comes_From_Source (N) then + if Comes_From_Source (Original_Node (N)) then if Restriction_Check_Required (SPARK) and then Is_In_Hidden_Part_In_SPARK (Sloc (N))
[Ada] Isolate variables used to handle exceptions during finalization
This is a small refactoring that remove some duplicate code. No functional change. Tested on x86_64-pc-linux-gnu, committed on trunk 2011-08-31 Tristan Gingold * exp_ch7.ads, exp_ch7.adb (Finalization_Exception_Data): New type to hold variables between these following subprograms. (Build_Exception_Handler, Build_Object_Declarations, Build_Raise_Statement): Use the above type as parameter. Make the above adjustments. * exp_intr.adb (Expand_Unc_Deallocation): Adjust. Index: exp_ch7.adb === --- exp_ch7.adb (revision 178360) +++ exp_ch7.adb (working copy) @@ -711,36 +711,35 @@ - function Build_Exception_Handler - (Loc : Source_Ptr; - E_Id: Entity_Id; - Raised_Id : Entity_Id; + (Data: Finalization_Exception_Data; For_Library : Boolean := False) return Node_Id is Actuals : List_Id; Proc_To_Call : Entity_Id; begin - pragma Assert (Present (E_Id)); - pragma Assert (Present (Raised_Id)); + pragma Assert (Present (Data.E_Id)); + pragma Assert (Present (Data.Raised_Id)); -- Generate: --Get_Current_Excep.all.all Actuals := New_List ( -Make_Explicit_Dereference (Loc, +Make_Explicit_Dereference (Data.Loc, Prefix => -Make_Function_Call (Loc, +Make_Function_Call (Data.Loc, Name => -Make_Explicit_Dereference (Loc, +Make_Explicit_Dereference (Data.Loc, Prefix => -New_Reference_To (RTE (RE_Get_Current_Excep), Loc); +New_Reference_To (RTE (RE_Get_Current_Excep), + Data.Loc); if For_Library and then not Restricted_Profile then Proc_To_Call := RTE (RE_Save_Library_Occurrence); else Proc_To_Call := RTE (RE_Save_Occurrence); - Prepend_To (Actuals, New_Reference_To (E_Id, Loc)); + Prepend_To (Actuals, New_Reference_To (Data.E_Id, Data.Loc)); end if; -- Generate: @@ -754,23 +753,23 @@ -- end if; return -Make_Exception_Handler (Loc, +Make_Exception_Handler (Data.Loc, Exception_Choices => -New_List (Make_Others_Choice (Loc)), +New_List (Make_Others_Choice (Data.Loc)), Statements => New_List ( -Make_If_Statement (Loc, +Make_If_Statement (Data.Loc, Condition => -Make_Op_Not (Loc, - Right_Opnd => New_Reference_To (Raised_Id, Loc)), +Make_Op_Not (Data.Loc, + Right_Opnd => New_Reference_To (Data.Raised_Id, Data.Loc)), Then_Statements => New_List ( -Make_Assignment_Statement (Loc, - Name => New_Reference_To (Raised_Id, Loc), - Expression => New_Reference_To (Standard_True, Loc)), +Make_Assignment_Statement (Data.Loc, + Name => New_Reference_To (Data.Raised_Id, Data.Loc), + Expression => New_Reference_To (Standard_True, Data.Loc)), -Make_Procedure_Call_Statement (Loc, +Make_Procedure_Call_Statement (Data.Loc, Name => -New_Reference_To (Proc_To_Call, Loc), +New_Reference_To (Proc_To_Call, Data.Loc), Parameter_Associations => Actuals); end Build_Exception_Handler; @@ -1052,21 +1051,14 @@ -- structures right from the start. Entities and lists are created once -- it has been established that N has at least one controlled object. - Abort_Id : Entity_Id := Empty; - -- Entity of local flag. The flag is set when finalization is triggered - -- by an abort. - Components_Built : Boolean := False; -- A flag used to avoid double initialization of entities and lists. If -- the flag is set then the following variables have been initialized: -- - --Abort_Id --Counter_Id - --E_Id --Finalizer_Decls --Finalizer_Stmts --Jump_Alts - --Raised_Id Counter_Id : Entity_Id := Empty; Counter_Val : Int := 0; @@ -1076,9 +1068,8 @@ -- Declarative region of N (if available). If N is a package declaration -- Decls denotes the visible declarations. - E_Id : Entity_Id := Empty; - -- Entity of the local exception occurence. The first exception which - -- occurred during finalization is stored in E_Id and later reraised. + Finalizer_Data : Finalization_Exception_Data; + -- Data for the exception Finalizer_Decls : List_Id := No_List; -- Local variable decl
[Ada] Correct various bad choices in Alfa mode
Tested on x86_64-pc-linux-gnu, committed on trunk 2011-08-31 Yannick Moy * exp_alfa.adb (Expand_Alfa_N_Package_Declaration, Expand_Alfa_N_Subprogram_Body): Remove useless procedures which simply call Qualify_Entity_Names. (Expand_Alfa): call Qualify_Entity_Names in more cases * lib-xref-alfa.adb: Take into account system package. * sem_prag.adb Take into account restrictions in Alfa mode, contrary to CodePeer mode in which we are interested in finding bugs even if compiler cannot compile source. * sem_util.adb, sem_util.ads (Unique_Entity): Take into account case of deferred constant. Index: exp_alfa.adb === --- exp_alfa.adb(revision 178360) +++ exp_alfa.adb(working copy) @@ -51,15 +51,9 @@ procedure Expand_Alfa_N_Attribute_Reference (N : Node_Id); -- Expand attributes 'Old and 'Result only - procedure Expand_Alfa_N_Package_Declaration (N : Node_Id); - -- Fully qualify names of enclosed entities - procedure Expand_Alfa_N_Simple_Return_Statement (N : Node_Id); -- Insert conversion on function return if necessary - procedure Expand_Alfa_N_Subprogram_Body (N : Node_Id); - -- Fully qualify names of enclosed entities - procedure Expand_Alfa_Simple_Function_Return (N : Node_Id); -- Expand simple return from function @@ -71,15 +65,15 @@ begin case Nkind (N) is - when N_Package_Declaration => -Expand_Alfa_N_Package_Declaration (N); + when N_Package_Body| + N_Package_Declaration | + N_Subprogram_Body | + N_Block_Statement => +Qualify_Entity_Names (N); when N_Simple_Return_Statement => Expand_Alfa_N_Simple_Return_Statement (N); - when N_Subprogram_Body => -Expand_Alfa_N_Subprogram_Body (N); - when N_Function_Call| N_Procedure_Call_Statement => Expand_Alfa_Call (N); @@ -173,15 +167,6 @@ end case; end Expand_Alfa_N_Attribute_Reference; - --- - -- Expand_Alfa_N_Package_Declaration -- - --- - - procedure Expand_Alfa_N_Package_Declaration (N : Node_Id) is - begin - Qualify_Entity_Names (N); - end Expand_Alfa_N_Package_Declaration; - --- -- Expand_Alfa_N_Simple_Return_Statement -- --- @@ -222,15 +207,6 @@ return; end Expand_Alfa_N_Simple_Return_Statement; - --- - -- Expand_Alfa_N_Subprogram_Body -- - --- - - procedure Expand_Alfa_N_Subprogram_Body (N : Node_Id) is - begin - Qualify_Entity_Names (N); - end Expand_Alfa_N_Subprogram_Body; - -- Expand_Alfa_Simple_Function_Return -- Index: sem_prag.adb === --- sem_prag.adb(revision 178358) +++ sem_prag.adb(working copy) @@ -5090,9 +5090,9 @@ -- Start of processing for Process_Restrictions_Or_Restriction_Warnings begin - -- Ignore all Restrictions pragma in CodePeer and Alfa modes + -- Ignore all Restrictions pragma in CodePeer mode - if CodePeer_Mode or Alfa_Mode then + if CodePeer_Mode then return; end if; Index: sem_util.adb === --- sem_util.adb(revision 178358) +++ sem_util.adb(working copy) @@ -12656,6 +12656,11 @@ begin case Ekind (E) is + when E_Constant => +if Present (Full_View (E)) then + U := Full_View (E); +end if; + when Type_Kind => if Present (Full_View (E)) then U := Full_View (E); Index: sem_util.ads === --- sem_util.ads(revision 178358) +++ sem_util.ads(working copy) @@ -1448,7 +1448,8 @@ -- views of the same entity have the same unique defining entity: -- * package spec and body; -- * subprogram declaration, subprogram stub and subprogram body; - -- * private view and full view of a type. + -- * private view and full view of a type; + -- * private view and full view of a deferred constant. -- In other cases, return the defining entity for N. function Unique_Entity (E : Entity_Id) return Entity_Id; Index: lib-xref-alfa.adb === --- lib-xref-alfa.adb (revision 178363) +++ lib-xref-alfa.adb (working copy) @@ -886,14 +886,7 @@ -- Generate file and scope Alfa infor
Re: Vector shuffling
On Wed, Aug 31, 2011 at 10:35 AM, Duncan Sands wrote: > Hi Artem, > > On 31/08/11 10:27, Artem Shinkarov wrote: >> >> On Wed, Aug 31, 2011 at 12:51 AM, Chris Lattner >> wrote: >>> >>> On Aug 30, 2011, at 10:01 AM, Artem Shinkarov wrote: >> >> The patch at the moment lacks of some examples, but mainly it works >> fine for me. It would be nice if i386 gurus could look into the way I >> am doing the expansion. >> >> Middle-end parts seems to be more or less fine, they have not changed >> much from the previous time. > > +@code{__builtin_shuffle (vec, mask)} and > +@code{__builtin_shuffle (vec0, vec1, mask)}. Both functions construct > > the latter would be __builtin_shuffle2. Why?? That was the syntax we agreed on that elegantly handles both cases in one place. >>> >>> If you're going to add vector shuffling builtins, you might consider >>> adding the same builtin that clang has for compatibility: >>> >>> http://clang.llvm.org/docs/LanguageExtensions.html#__builtin_shufflevector >>> >>> It should be straight-forward to map it into the same IR. >>> >>> -Chris >>> >> >> Chris >> >> I am trying to use OpenCL syntax here which says that the mask for >> shuffling is a vector. Also I didn't really get from the clang >> description if the indexes could be non-constnants? If not, then I >> have a problem here, because I want to support this. > > probably it maps directly to the LLVM shufflevector instruction, see > http://llvm.org/docs/LangRef.html#i_shufflevector > That requires the shuffle mask to be constant. I see. I think it's not worth copying LLVM builtins that merely map its internal IL. Richard. > Ciao, Duncan. >
[Ada] Initialize pointer components of red-black tree node
When a node is allocated from the free store, its pointer components (the links to other nodes in the tree) must also be initialized (to 0, the equivalent of null). This simplifies the post-allocation handling of nodes inserted into terminal positions. Tested on x86_64-pc-linux-gnu, committed on trunk 2011-08-31 Matthew Heaney * a-rbtgbo.adb (Generic_Allocate): Initialize pointer components of node to null value. Index: a-rbtgbo.adb === --- a-rbtgbo.adb(revision 178358) +++ a-rbtgbo.adb(working copy) @@ -6,7 +6,7 @@ -- -- -- B o d y -- -- -- --- Copyright (C) 2004-2010, Free Software Foundation, Inc. -- +-- Copyright (C) 2004-2011, Free Software Foundation, Inc. -- -- -- -- GNAT is free software; you can redistribute it and/or modify it under -- -- terms of the GNU General Public License as published by the Free Soft- -- @@ -586,6 +586,10 @@ Set_Element (N (Node)); Tree.Free := Tree.Free - 1; end if; + + Set_Parent (N (Node), Parent => 0); + Set_Left (N (Node), Left => 0); + Set_Right (N (Node), Right => 0); end Generic_Allocate; ---
[Ada] Ambiguities with prefixed views of synchronized primitives
A selected_component whose selector_name denotes an entity of a concurrent tagged type may be ambiguous because the target entity may be covered by a class-wide subprogram. This patch adds this missing test to the frontend to report the ambiguity. The following test must now compile with errors: package Synch_Pkg2 is type Synch_Interface is synchronized interface; procedure Yet_Another_Op (Obj : in out Synch_Interface'Class); end Synch_Pkg2; with Synch_Pkg2; package Task_Pkg2 is task type Task_Type is new Synch_Pkg2.Synch_Interface with entry Yet_Another_Op; end Task_Type; end Task_Pkg2; with Synch_Pkg2; use Synch_Pkg2; with Task_Pkg2; procedure ai05_0090 is T : Task_Pkg2.Task_Type; begin T.Yet_Another_Op; -- (3) Ambiguous? (Yes.) end; Command: gcc -c -gnat05 ai05_0090.adb Output: ai05_0090.adb:7:05: ambiguous call to "Yet_Another_Op" ai05_0090.adb:7:05: possible interpretation at task_pkg2.ads:5 ai05_0090:7:05: possible interpretation at synch_pkg2.ads:3 Tested on x86_64-pc-linux-gnu, committed on trunk 2011-08-31 Javier Miranda * sem_ch4.adb (Try_Object_Operation): Addition of one formal to search only for class-wide subprograms conflicting with entities of concurrent tagged types. Index: sem_ch4.adb === --- sem_ch4.adb (revision 178361) +++ sem_ch4.adb (working copy) @@ -276,11 +276,16 @@ -- subprogram, and the call F (X) interpreted as F.all (X). In this case -- the call may be overloaded with both interpretations. - function Try_Object_Operation (N : Node_Id) return Boolean; + function Try_Object_Operation + (N : Node_Id; CW_Test_Only : Boolean := False) return Boolean; -- Ada 2005 (AI-252): Support the object.operation notation. If node N -- is a call in this notation, it is transformed into a normal subprogram -- call where the prefix is a parameter, and True is returned. If node - -- N is not of this form, it is unchanged, and False is returned. + -- N is not of this form, it is unchanged, and False is returned. if + -- CW_Test_Only is true then N is an N_Selected_Component node which + -- is part of a call to an entry or procedure of a tagged concurrent + -- type and this routine is invoked to search for class-wide subprograms + -- conflicting with the target entity. procedure wpo (T : Entity_Id); pragma Warnings (Off, wpo); @@ -4165,6 +4170,25 @@ then return; end if; + +-- Ada 2012 (AI05-0090-1): If we found a candidate of a call to an +-- entry or procedure of a tagged concurrent type we must check +-- if there are class-wide subprograms covering the primitive. If +-- true then Try_Object_Operation reports the error. + +if Has_Candidate + and then Is_Concurrent_Type (Prefix_Type) + and then Nkind (Parent (N)) = N_Procedure_Call_Statement + + -- Duplicate the call. This is required to avoid problems with + -- the tree transformations performed by Try_Object_Operation. + + and then Try_Object_Operation + (N => Sinfo.Name (New_Copy_Tree (Parent (N))), + CW_Test_Only => True) +then + return; +end if; end if; if Etype (N) = Any_Type and then Is_Protected_Type (Prefix_Type) then @@ -6609,7 +6633,9 @@ -- Try_Object_Operation -- -- - function Try_Object_Operation (N : Node_Id) return Boolean is + function Try_Object_Operation + (N : Node_Id; CW_Test_Only : Boolean := False) return Boolean + is K : constant Node_Kind := Nkind (Parent (N)); Is_Subprg_Call : constant Boolean:= Nkind_In (K, N_Procedure_Call_Statement, @@ -6898,14 +6924,17 @@ -- procedure Report_Ambiguity (Op : Entity_Id) is - Access_Formal : constant Boolean := - Is_Access_Type (Etype (First_Formal (Op))); Access_Actual : constant Boolean := Is_Access_Type (Etype (Prefix (N))); + Access_Formal : Boolean := False; begin Error_Msg_Sloc := Sloc (Op); + if Present (First_Formal (Op)) then +Access_Formal := Is_Access_Type (Etype (First_Formal (Op))); + end if; + if Access_Formal and then not Access_Actual then if Nkind (Parent (Op)) = N_Full_Type_Declaration then Error_Msg_N @@ -7205,6 +7234,13 @@ -- Start of processing for Try_Class_Wide_Operation begin + -- If we are searching only for conflicting class-wide subprograms + -- then initialize directly Matching_Op with the target entity. + + if CW_Test_Only then +
[Ada] Default-initialize Nodes component
When manipulating bounded tree-based container objects, that do not otherwise have any explicit initialization expression, the compiler would emit a warning about the object not being initialized, because the Nodes component of the tree type had not been given an initialization expression. This warning is a false positive, because the logical state of the object is empty, and other components of the tree record type are initialized, in a manner that establishes the representation invariant of the object. In order to eliminate the warning, the Nodes component of the tree type was given an initialization expression. Tested on x86_64-pc-linux-gnu, committed on trunk 2011-08-31 Matthew Heaney * a-crbltr.ads (Tree_Type): Default-initialize the Nodes component. Index: a-crbltr.ads === --- a-crbltr.ads(revision 178358) +++ a-crbltr.ads(working copy) @@ -6,7 +6,7 @@ -- -- -- S p e c -- -- -- --- Copyright (C) 2004-2010, Free Software Foundation, Inc. -- +-- Copyright (C) 2004-2011, Free Software Foundation, Inc. -- -- -- -- GNAT is free software; you can redistribute it and/or modify it under -- -- terms of the GNU General Public License as published by the Free Soft- -- @@ -53,6 +53,13 @@ package Generic_Bounded_Tree_Types is type Nodes_Type is array (Count_Type range <>) of Node_Type; + -- Note that objects of type Tree_Type are logically initialized (in the + -- sense that representation invariants of type are satisfied by dint of + -- default initialization), even without the Nodes component also having + -- its own initialization expression. We only initializae the Nodes + -- component here in order to prevent spurious compiler warnings about + -- the container object not being fully initialized. + type Tree_Type (Capacity : Count_Type) is tagged record First : Count_Type := 0; Last : Count_Type := 0; @@ -61,7 +68,7 @@ Busy : Natural := 0; Lock : Natural := 0; Free : Count_Type'Base := -1; - Nodes : Nodes_Type (1 .. Capacity); + Nodes : Nodes_Type (1 .. Capacity) := (others => <>); end record; end Generic_Bounded_Tree_Types;
[Ada] Error on instantiation with private type that has discriminated full type
The compiler incorrectly rejects a generic instantiation that passes an undiscriminated private type when the full type has discriminants and the generic does an assignment to a dereferenced access value denoting an object of the formal type. The error message complains that the type has no discriminants because, in the instance, the compiler expands a record subtype whose base type is the actual private type, though the underlying type is a discriminated record type. It turns out to be problematic to change the subtype to have the record type as its base type (breaks lots of existing tests), and the fix adopted is to add a test to go to the underlying type in this case prior to expanding the discriminant check for this already specially handled form of assignment. The following test must compile and execute quietly: procedure Priv_Discrim_Inst_Bug is generic type Data is private; package Gen is procedure Assign (X: in Data); private type Acc_Data is access all Data; Default_Object : aliased Data; end Gen; package body Gen is procedure Assign (X: in Data) is A : constant Acc_Data := Default_Object'Access; begin A.all := X; -- Discriminant check required for instance Inst end Assign; end Gen; package Pkg is type Priv is private; function Return_Priv (B : Boolean) return Priv; private type Priv (Discr : Boolean := True) is null record; end Pkg; package body Pkg is function Return_Priv (B : Boolean) return Priv is begin return (Discr => B); end Return_Priv; end Pkg; package Inst is new Gen (Pkg.Priv); -- OK (but GNAT says no discriminants) begin begin Inst.Assign (Pkg.Return_Priv (False)); -- Should raise exception exception when others => null; end; Inst.Assign (Pkg.Return_Priv (True)); -- Should not raise exception end Priv_Discrim_Inst_Bug; Tested on x86_64-pc-linux-gnu, committed on trunk 2011-08-31 Gary Dismukes * exp_ch5.adb (Expand_N_Assignment_Statement): When a discriminant check is needed for a left-hand side that is a dereference, and the base type is private without discriminants (whereas the full type does have discriminants), an extra retrieval of the underlying type may be needed in the case where the subtype is a record subtype whose base type is private. Update comments. Index: exp_ch5.adb === --- exp_ch5.adb (revision 178361) +++ exp_ch5.adb (working copy) @@ -1788,9 +1788,8 @@ -- If the type is private without discriminants, and the full type -- has discriminants (necessarily with defaults) a check may still be - -- necessary if the Lhs is aliased. The private determinants must be + -- necessary if the Lhs is aliased. The private discriminants must be -- visible to build the discriminant constraints. - -- What is a "determinant"??? -- Only an explicit dereference that comes from source indicates -- aliasing. Access to formals of protected operations and entries @@ -1802,11 +1801,28 @@ and then Comes_From_Source (Lhs) then declare -Lt : constant Entity_Id := Etype (Lhs); +Lt : constant Entity_Id := Etype (Lhs); +Ubt : Entity_Id := Base_Type (Typ); + begin -Set_Etype (Lhs, Typ); -Rewrite (Rhs, OK_Convert_To (Base_Type (Typ), Rhs)); -Apply_Discriminant_Check (Rhs, Typ, Lhs); +-- In the case of an expander-generated record subtype whose base +-- type still appears private, Typ will have been set to that +-- private type rather than the underlying record type (because +-- Underlying type will have returned the record subtype), so it's +-- necessary to apply Underlying_Type again to the base type to +-- get the record type we need for the discriminant check. Such +-- subtypes can be created for assignments in certain cases, such +-- as within an instantiation passed this kind of private type. +-- It would be good to avoid this special test, but making changes +-- to prevent this odd form of record subtype seems difficult. ??? + +if Is_Private_Type (Ubt) then + Ubt := Underlying_Type (Ubt); +end if; + +Set_Etype (Lhs, Ubt); +Rewrite (Rhs, OK_Convert_To (Base_Type (Ubt), Rhs)); +Apply_Discriminant_Check (Rhs, Ubt, Lhs); Set_Etype (Lhs, Lt); end;
[PATCH] Remove bogus TYPE_IS_SIZETYPE special-casing in extract_muldiv_1
When making sizetypes no longer sign-extended (they are unsigned) we run into extract_muldiv_1 miscompiling the Ada RTS during secondary stack initialization while folding sizes for an allocation. From ((sizetype) (_GLOBAL.SZ4_system.secondary_stack (.last, .first) /[cl] 8) + 15 & 0x0fff0) + 32 we eventually generate 2305843009213704224. Oops. This is because extract_multiv_1 happily transforms (((10240 - (sizetype) first) + 1) * 8) /[cl] 8 through ((sizetype) first * 0x0fff8 + 81928) /[cl] 8 to ((sizetype) first * 2305843009213693951 + 10241) and then substitute 1 for first. Well, the comment for that folding is totally odd - of _course_ unsigned sizetype things can overflow (we hid that issue merely by pretending all unsigned sizetype constants (yes, only constants) are signed. Huh.) Off it goes. Bootstrap and regtest pending on x86_64-unknown-linux-gnu. Richard. 2011-08-31 Richard Guenther * fold-const.c (extract_muldiv_1): Remove bogus TYPE_IS_SIZETYPE special-casing. Index: trunk/gcc/fold-const.c === --- trunk.orig/gcc/fold-const.c 2011-08-31 10:53:58.0 +0200 +++ trunk/gcc/fold-const.c 2011-08-31 10:45:09.0 +0200 @@ -5894,11 +5894,9 @@ extract_muldiv_1 (tree t, tree c, enum t multiple of the other, in which case we replace this with either an operation or CODE or TCODE. -If we have an unsigned type that is not a sizetype, we cannot do -this since it will change the result if the original computation -overflowed. */ - if ((TYPE_OVERFLOW_UNDEFINED (ctype) - || (TREE_CODE (ctype) == INTEGER_TYPE && TYPE_IS_SIZETYPE (ctype))) +If we have an unsigned type, we cannot do this since it will change +the result if the original computation overflowed. */ + if (TYPE_OVERFLOW_UNDEFINED (ctype) && ((code == MULT_EXPR && tcode == EXACT_DIV_EXPR) || (tcode == MULT_EXPR && code != TRUNC_MOD_EXPR && code != CEIL_MOD_EXPR
[Ada] Insert must check for zero-length buckets array
Hash values must be folded by the length of the buckets array. The length of the buckets array is a discriminant of the container type, which means that it is possible for the user to specify 0 as the length value. Insert must therefore check for this case specifically, in order to prevent divide-by-zero errors that would otherwise occur during computation of the bucket index for a given key. Tested on x86_64-pc-linux-gnu, committed on trunk 2011-08-31 Matthew Heaney * a-cbhama.adb, a-cbhase.adb (Insert): Check for zero-length buckets array. Index: a-cbhama.adb === --- a-cbhama.adb(revision 178358) +++ a-cbhama.adb(working copy) @@ -513,6 +513,11 @@ procedure Assign_Key (Node : in out Node_Type) is begin Node.Key := Key; + + -- Note that we do not also assign the element component of the node + -- here, because this version of Insert does not accept an element + -- parameter. + -- Node.Element := New_Item; end Assign_Key; @@ -530,20 +535,17 @@ -- Start of processing for Insert begin - -- ??? - -- if HT_Ops.Capacity (HT) = 0 then - -- HT_Ops.Reserve_Capacity (HT, 1); - -- end if; + -- The buckets array length is specified by the user as a discriminant + -- of the container type, so it is possible for the buckets array to + -- have a length of zero. We must check for this case specifically, in + -- order to prevent divide-by-zero errors later, when we compute the + -- buckets array index value for a key, given its hash value. - Local_Insert (Container, Key, Position.Node, Inserted); + if Container.Buckets'Length = 0 then + raise Capacity_Error with "No capacity for insertion"; + end if; - -- ??? - -- if Inserted - --and then HT.Length > HT_Ops.Capacity (HT) - -- then - -- HT_Ops.Reserve_Capacity (HT, HT.Length); - -- end if; - + Local_Insert (Container, Key, Position.Node, Inserted); Position.Container := Container'Unchecked_Access; end Insert; @@ -590,20 +592,17 @@ -- Start of processing for Insert begin - -- ?? - -- if HT_Ops.Capacity (HT) = 0 then - -- HT_Ops.Reserve_Capacity (HT, 1); - -- end if; + -- The buckets array length is specified by the user as a discriminant + -- of the container type, so it is possible for the buckets array to + -- have a length of zero. We must check for this case specifically, in + -- order to prevent divide-by-zero errors later, when we compute the + -- buckets array index value for a key, given its hash value. - Local_Insert (Container, Key, Position.Node, Inserted); + if Container.Buckets'Length = 0 then + raise Capacity_Error with "No capacity for insertion"; + end if; - -- ??? - -- if Inserted - --and then HT.Length > HT_Ops.Capacity (HT) - -- then - -- HT_Ops.Reserve_Capacity (HT, HT.Length); - -- end if; - + Local_Insert (Container, Key, Position.Node, Inserted); Position.Container := Container'Unchecked_Access; end Insert; Index: a-cbhase.adb === --- a-cbhase.adb(revision 178358) +++ a-cbhase.adb(working copy) @@ -710,19 +710,17 @@ -- Start of processing for Insert begin - -- ??? - -- if HT_Ops.Capacity (HT) = 0 then - -- HT_Ops.Reserve_Capacity (HT, 1); - -- end if; + -- The buckets array length is specified by the user as a discriminant + -- of the container type, so it is possible for the buckets array to + -- have a length of zero. We must check for this case specifically, in + -- order to prevent divide-by-zero errors later, when we compute the + -- buckets array index value for an element, given its hash value. - Local_Insert (Container, New_Item, Node, Inserted); + if Container.Buckets'Length = 0 then + raise Capacity_Error with "No capacity for insertion"; + end if; - -- ??? - -- if Inserted - --and then HT.Length > HT_Ops.Capacity (HT) - -- then - -- HT_Ops.Reserve_Capacity (HT, HT.Length); - -- end if; + Local_Insert (Container, New_Item, Node, Inserted); end Insert; --
[Ada] Check ambiguity with prefixed views of tagged primitives
When the frontend resolves a dispatching call through the object operation notation it must also check if there is a class-wide subprogram covering the target primitive. This check was missing in the frontend. After this patch the following test must compile with errors: package Pkg1 is type Iface is interface; procedure Yet_Another_Op (Obj : in out Iface'Class); end; with Pkg1; package Pkg2 is type Typ is new Pkg1.Iface with null record; procedure Yet_Another_Op (Obj : in out Typ); end; with Pkg1; use Pkg1; with Pkg2; use Pkg2; procedure Main is T : Pkg2.Typ; begin T.Yet_Another_Op; -- Ambiguous? (Yes) end; Command: gcc -c -gnat05 main.adb Output: main.adb:7:05: ambiguous expression (cannot resolve "Yet_Another_Op") main.adb:7:05: possible interpretation at pkg2.ads:6 main.adb:7:05: possible interpretation at pkg1.ads:3 Tested on x86_64-pc-linux-gnu, committed on trunk 2011-08-31 Javier Miranda * sem_ch4.adb (Try_Object_Operation): When a dispatching primitive is found check if there is a class-wide subprogram covering the primitive. Index: sem_ch4.adb === --- sem_ch4.adb (revision 178360) +++ sem_ch4.adb (working copy) @@ -6638,7 +6638,7 @@ Call: Node_Id; Subp: Entity_Id) return Entity_Id; -- If the subprogram is a valid interpretation, record it, and add - -- to the list of interpretations of Subprog. + -- to the list of interpretations of Subprog. Otherwise return Empty. procedure Complete_Object_Operation (Call_Node : Node_Id; @@ -7104,6 +7104,14 @@ and then N = Name (Parent (N)) then goto Next_Hom; + + -- If the context is a function call, ignore procedures + -- in the name of the call. + + elsif Ekind (Hom) = E_Procedure +and then Nkind (Parent (N)) /= N_Procedure_Call_Statement + then + goto Next_Hom; end if; Set_Etype (Call_Node, Any_Type); @@ -7271,16 +7279,39 @@ return; end if; - if Try_Primitive_Operation - (Call_Node => New_Call_Node, - Node_To_Replace => Node_To_Replace) - or else - Try_Class_Wide_Operation - (Call_Node => New_Call_Node, -Node_To_Replace => Node_To_Replace) - then -null; - end if; + declare +Dup_Call_Node : constant Node_Id := New_Copy (New_Call_Node); +CW_Result : Boolean; +Prim_Result : Boolean; +pragma Unreferenced (CW_Result); + + begin +Prim_Result := + Try_Primitive_Operation +(Call_Node => New_Call_Node, + Node_To_Replace => Node_To_Replace); + +-- Check if there is a class-wide subprogram covering the +-- primitive. This check must be done even if a candidate +-- was found in order to report ambiguous calls. + +if not (Prim_Result) then + CW_Result := + Try_Class_Wide_Operation + (Call_Node => New_Call_Node, +Node_To_Replace => Node_To_Replace); + +-- If we found a primitive we search for class-wide subprograms +-- using a duplicate of the call node (done to avoid missing its +-- decoration if there is no ambiguity). + +else + CW_Result := + Try_Class_Wide_Operation + (Call_Node => Dup_Call_Node, +Node_To_Replace => Node_To_Replace); +end if; + end; end Try_One_Prefix_Interpretation; -
[Ada] Filter out read reference to operator in Alfa xrefs
Alfa cross references should only contain 'call' references for operators. Filter out unwanted references which are otherwise generated on intrinsic operator. Tested on x86_64-pc-linux-gnu, committed on trunk 2011-08-31 Yannick Moy * lib-xref-alfa.adb (Add_Alfa_Xrefs): Do not take into account read reference to operator in Alfa xrefs. Index: lib-xref-alfa.adb === --- lib-xref-alfa.adb (revision 178358) +++ lib-xref-alfa.adb (working copy) @@ -576,6 +576,11 @@ Eliminate_Before_Sort : declare NR : Nat; + function Is_Alfa_Reference + (E : Entity_Id; +Typ : Character) return Boolean; + -- Return whether the reference is adequate for this entity + function Is_Alfa_Scope (E : Entity_Id) return Boolean; -- Return whether the entity or reference scope is adequate @@ -583,6 +588,25 @@ -- Return True if E is a global constant for which we should ignore -- reads in Alfa. + --- + -- Is_Alfa_Reference -- + --- + + function Is_Alfa_Reference + (E : Entity_Id; +Typ : Character) return Boolean is + begin +-- The only references of interest on callable entities are calls. +-- On non-callable entities, the only references of interest are +-- reads and writes. + +if Ekind (E) in Overloadable_Kind then + return Typ = 's'; +else + return Typ = 'r' or else Typ = 'm'; +end if; + end Is_Alfa_Reference; + --- -- Is_Alfa_Scope -- --- @@ -617,6 +641,8 @@ and then Is_Alfa_Scope (Xrefs.Table (Rnums (J)).Ent_Scope) and then Is_Alfa_Scope (Xrefs.Table (Rnums (J)).Ref_Scope) and then not Is_Global_Constant (Xrefs.Table (Rnums (J)).Ent) + and then Is_Alfa_Reference (Xrefs.Table (Rnums (J)).Ent, + Xrefs.Table (Rnums (J)).Typ) then Nrefs := Nrefs + 1; Rnums (Nrefs) := Rnums (J);
[Ada] Crash on quantified expressions containing 'Old in postconditions
The attribute reference 'Old ppears within a postcondition, but refers to an entity in the enclosing subprogram. If the prefix is a component of a formal its expansion might generate actual subtypes that may be referenced in an inner context, and which must be elaborated within the subprogram itself. As a result we create a declaration for the prefix, insert it in the enclosing subprogram itself and expand it at once. This is properly an expansion activity but it has to be performed now to prevent out-of-order issues in the back-end. the following commands gnatmake -q -gnat12 -gnata my_test my_test must yield: Max = 9 Min = 0 Average = 4 Find 0 in T1 4 Find 9 in T2 4 --- with Simple_Unc_Arrays; use Simple_Unc_Arrays; with Ada.Text_IO; use Ada.Text_IO; procedure My_Test is T1 : Table := (10, (5, 1, 3, 0, 9, 8, 2, 7, 4, 6)); T2 : Table := (10, (4, 8, 6, 9, 0, 1, 7, 2, 5, 3)); T3 : Table (10); begin Put_Line ("Max = " & Max (T1)'img); Put_Line ("Min = " & Min (T1)'img ); Put_Line ("Average = " & Average (T1)'img); Put_Line ("Find 0 in T1 " & Search (T1, 0)'img); Put_Line ("Find 9 in T2 " & Search (T2, 9)'img); T3 := Add (T1, T2); pragma Assert (for all I in 1 .. T3.Last => T3.V (I) = 9); T3 := Bubble_Sort (T1); Quick_Sort (T1); pragma Assert (T1 = T3); end; --- package Simple_Unc_Arrays is -- This should be a generic parameter type Value is new Integer; -- should be range <>; -- This is declaring arrays with a fixed first bound of 1 type Values is array (Positive range <>) of Value; type Table (Last : Natural) is record V : Values (1 .. Last); end record; - -- Add -- - function Add (A, B : Table) return Table with Pre => Same_Range (A, B), Post => Same_Range (Add'Result, A) and then (for all J in 1 .. A.Last => (Add'Result.V (J) = A.V (J) + B.V (J))); - -- Reverse -- - Procedure Inverse (A : in out Table) with Post => (for all J in 1 .. A.Last => (A.V (J) = A.V'Old (A.Last - J + 1))); -- Reverses the content of A - -- Min -- - function Min (A : Table) return Value with Pre => not Empty (A), Post => (for all J in 1 .. A.Last => Min'Result <= A.V (J)) and then (for some J in 1 .. A.Last => Min'Result = A.V (J)); -- Returns the minimum value occurring in A - -- Max -- - function Max (A : Table) return Value with Pre => not Empty (A), Post =>(for all J in 1 .. A.Last => Max'Result >= A.V (J)) and then (for some J in 1 .. A.Last => Max'Result = A.V (J)); -- Returns the maximun value occurring in A - -- Average -- - function Average (A : Table) return Value with Pre => not Empty (A), Post =>Min (A) <= Average'Result and then Max (A) >= Average'Result; -- Returns the average value in A. Here, the given postcondition does not -- describe completely the value returned. -- Search -- function Search (A : Table; V : Value) return Natural with Post => (Search'Result = 0 and then Not_In (A, V, 1 ,A.Last)) or else (A.V (Search'Result) = V and then Not_In (A, V, 1, Search'Result-1)); -- Returns the first index at which value V occurs in array A, or else 0 - -- Bubble_Sort -- - function Bubble_Sort (A: Table) return Table with Post => (for all J in 1 .. A.Last - 1 => Bubble_Sort'Result.V (J) <= Bubble_Sort'Result.V (J+1)) and then (for all J in 1 .. A.Last => (for some K in 1 .. A.Last => A.V (J) = Bubble_Sort'Result.V (K))); -- Sorts an array in increasing order by using bubble sort -- Quick_Sort -- Procedure Quick_Sort (A: in out Table); -- with Post => (for all J in 1 .. A.Last - 1 => A.V (J) <= A.V (J+1)) --and then (for all J in 1 .. A.Last => --(for some K in 1 .. A.Last => A.V'Old (J) = A.V (K))); -- Sorts an array in increasing order by using quick sort - -- utililities -- - function Empty (A : Table) return Boolean is (1 > A.Last); function Same_Range (A, B : Table) return Boolean is (A.Last = B.Last); function Not_In (A : Table; V : Value; Low : Positive; Up : Natural) return Boolean is (Up > A.Last or else (for all J in Low .. Up => A.V (J) /= V)); procedure Swap (V, W : in out Value) with Post => (V = W'Old and then W = V'Old); -- Swaps two values V and W end Simple_Unc_Arrays; --- package body Simple_Unc_Arrays is - -- Add -- - function Add (A, B : Table) return Table is begin return C : Table (A.Last) do
[Ada] Compiler crash on entry requeue in discriminated protected type
When compiling with -gnatc (or any mode that turns off the expansion phase), the compiler gets an Assertion_Failure when attempting to retrieve the body discriminal of a discriminant of a protected type. In the absence of expansion, a concurrent record type is not created, so this possibility has to be accounted for when retrieving body discriminals. The following test must compile quietly with -gnatc: procedure Requeue_Bug is type Boolean_Array is array (Positive range <>) of Boolean; protected type Protected_Type (Length : Positive) is entry Wait (Positive range 1 .. Length) (New_State : in Boolean); entry Wait_Next (New_State : in Boolean); private Available : Boolean_Array (1 .. Length) := (others => False); end Protected_Type; protected body Protected_Type is entry Wait (for J in Positive range 1 .. Length) (New_State : in Boolean) when Available (J) is begin null; end Wait; entry Wait_Next (New_State : in Boolean) when True is begin for J in 1 .. Length loop if not Available (J) then requeue Wait (J); end if; end loop; end Wait_Next; end Protected_Type; begin null; end Requeue_Bug; Tested on x86_64-pc-linux-gnu, committed on trunk 2011-08-31 Gary Dismukes * sem_util.adb (Find_Body_Discriminal): Test whether the scope of the spec discriminant is already a concurrent type, in which case just use it, otherwise fetch the Corresponding_Concurrent_Type as before. Index: sem_util.adb === --- sem_util.adb(revision 178355) +++ sem_util.adb(working copy) @@ -3701,13 +3701,22 @@ function Find_Body_Discriminal (Spec_Discriminant : Entity_Id) return Entity_Id is - pragma Assert (Is_Concurrent_Record_Type (Scope (Spec_Discriminant))); - - Tsk : constant Entity_Id := - Corresponding_Concurrent_Type (Scope (Spec_Discriminant)); + Tsk : Entity_Id; Disc : Entity_Id; begin + -- If expansion is suppressed, then the scope can be the concurrent type + -- itself rather than a corresponding concurrent record type. + + if Is_Concurrent_Type (Scope (Spec_Discriminant)) then + Tsk := Scope (Spec_Discriminant); + + else + pragma Assert (Is_Concurrent_Record_Type (Scope (Spec_Discriminant))); + + Tsk := Corresponding_Concurrent_Type (Scope (Spec_Discriminant)); + end if; + -- Find discriminant of original concurrent type, and use its current -- discriminal, which is the renaming within the task/protected body.
[Ada] Dereference correct hash table for a given node array index
Symmetric_Difference iterates over each hash table, populating the result set as it identifies items in one set not in the other set. The iterator works by passing a node index value back to the caller, which it in turn uses to dereference the node array. Both the Left and Right sets are visible in the scope of the iterator. The problem was that when iterating over the Right set, the index value passed back to the caller was used to dereference to the Left set, but this is not the correct array. The fix is to use the Right index to dereference the Right node array. Tested on x86_64-pc-linux-gnu, committed on trunk 2011-08-31 Matthew Heaney * a-cbhase.adb (Symmetric_Difference): Dereference correct node array. * a-chtgbo.adb (Free): Allow 0 as index value. Index: a-cbhase.adb === --- a-cbhase.adb(revision 178355) +++ a-cbhase.adb(working copy) @@ -1274,7 +1274,7 @@ - procedure Process (R_Node : Count_Type) is - N : Node_Type renames Left.Nodes (R_Node); + N : Node_Type renames Right.Nodes (R_Node); X : Count_Type; B : Boolean; Index: a-chtgbo.adb === --- a-chtgbo.adb(revision 178355) +++ a-chtgbo.adb(working copy) @@ -136,15 +136,19 @@ (HT : in out Hash_Table_Type'Class; X : Count_Type) is - pragma Assert (X > 0); + N : Nodes_Type renames HT.Nodes; + + begin + if X = 0 then + return; + end if; + pragma Assert (X <= HT.Capacity); - N : Nodes_Type renames HT.Nodes; -- pragma Assert (N (X).Prev >= 0); -- node is active -- Find a way to mark a node as active vs. inactive; we could -- use a special value in Color_Type for this. ??? - begin -- The hash table actually contains two data structures: a list for -- the "active" nodes that contain elements that have been inserted -- onto the container, and another for the "inactive" nodes of the free
Re: Vector shuffling
Hi Artem, On 31/08/11 10:27, Artem Shinkarov wrote: On Wed, Aug 31, 2011 at 12:51 AM, Chris Lattner wrote: On Aug 30, 2011, at 10:01 AM, Artem Shinkarov wrote: The patch at the moment lacks of some examples, but mainly it works fine for me. It would be nice if i386 gurus could look into the way I am doing the expansion. Middle-end parts seems to be more or less fine, they have not changed much from the previous time. +@code{__builtin_shuffle (vec, mask)} and +@code{__builtin_shuffle (vec0, vec1, mask)}. Both functions construct the latter would be __builtin_shuffle2. Why?? That was the syntax we agreed on that elegantly handles both cases in one place. If you're going to add vector shuffling builtins, you might consider adding the same builtin that clang has for compatibility: http://clang.llvm.org/docs/LanguageExtensions.html#__builtin_shufflevector It should be straight-forward to map it into the same IR. -Chris Chris I am trying to use OpenCL syntax here which says that the mask for shuffling is a vector. Also I didn't really get from the clang description if the indexes could be non-constnants? If not, then I have a problem here, because I want to support this. probably it maps directly to the LLVM shufflevector instruction, see http://llvm.org/docs/LangRef.html#i_shufflevector That requires the shuffle mask to be constant. Ciao, Duncan.
Re: Vector shuffling
On Wed, Aug 31, 2011 at 12:51 AM, Chris Lattner wrote: > On Aug 30, 2011, at 10:01 AM, Artem Shinkarov wrote: The patch at the moment lacks of some examples, but mainly it works fine for me. It would be nice if i386 gurus could look into the way I am doing the expansion. Middle-end parts seems to be more or less fine, they have not changed much from the previous time. >>> >>> +@code{__builtin_shuffle (vec, mask)} and >>> +@code{__builtin_shuffle (vec0, vec1, mask)}. Both functions construct >>> >>> the latter would be __builtin_shuffle2. >> >> Why?? >> That was the syntax we agreed on that elegantly handles both cases in one >> place. > > If you're going to add vector shuffling builtins, you might consider adding > the same builtin that clang has for compatibility: > http://clang.llvm.org/docs/LanguageExtensions.html#__builtin_shufflevector > > It should be straight-forward to map it into the same IR. > > -Chris > Chris I am trying to use OpenCL syntax here which says that the mask for shuffling is a vector. Also I didn't really get from the clang description if the indexes could be non-constnants? If not, then I have a problem here, because I want to support this. Artem.
Re: Vector shuffling
On Wed, Aug 31, 2011 at 1:51 AM, Chris Lattner wrote: > On Aug 30, 2011, at 10:01 AM, Artem Shinkarov wrote: The patch at the moment lacks of some examples, but mainly it works fine for me. It would be nice if i386 gurus could look into the way I am doing the expansion. Middle-end parts seems to be more or less fine, they have not changed much from the previous time. >>> >>> +@code{__builtin_shuffle (vec, mask)} and >>> +@code{__builtin_shuffle (vec0, vec1, mask)}. Both functions construct >>> >>> the latter would be __builtin_shuffle2. >> >> Why?? >> That was the syntax we agreed on that elegantly handles both cases in one >> place. > > If you're going to add vector shuffling builtins, you might consider adding > the same builtin that clang has for compatibility: > http://clang.llvm.org/docs/LanguageExtensions.html#__builtin_shufflevector > > It should be straight-forward to map it into the same IR. Sure. It doesn't support a vector argument for element selection though, which I think is required for a mapping to OpenCL shuffle/shuffle2. That's odd. Richard. > -Chris >
Re: Vector shuffling
On Tue, Aug 30, 2011 at 7:01 PM, Artem Shinkarov wrote: > On Tue, Aug 30, 2011 at 2:03 PM, Richard Guenther > wrote: >> On Tue, Aug 30, 2011 at 4:31 AM, Artem Shinkarov >> wrote: >>> Hi >>> >>> This is a patch for the explicit vector shuffling we have discussed a >>> long time ago here: >>> http://gcc.gnu.org/ml/gcc-patches/2010-08/msg01092.html >>> >>> The new patch introduces the new tree code, as we agreed, and expands >>> this code by checking the vshuffle pattern in the backend. >>> >>> The patch at the moment lacks of some examples, but mainly it works >>> fine for me. It would be nice if i386 gurus could look into the way I >>> am doing the expansion. >>> >>> Middle-end parts seems to be more or less fine, they have not changed >>> much from the previous time. >> >> +@code{__builtin_shuffle (vec, mask)} and >> +@code{__builtin_shuffle (vec0, vec1, mask)}. Both functions construct >> >> the latter would be __builtin_shuffle2. > > Why?? > That was the syntax we agreed on that elegantly handles both cases in one > place. Ah, then there was a case below that mentions __builtin_shuffle2 that needs adjusting then. >> +bool >> +expand_vec_shuffle_expr_p (enum machine_mode mode, tree v0, >> + tree v1, tree mask) >> +{ >> +#define inner_type_size(vec) \ >> + GET_MODE_BITSIZE (TYPE_MODE (TREE_TYPE (TREE_TYPE (vec >> >> missing comment. No #defines like this please, just initialize >> two temporary variables. >> >> + >> +rtx >> +expand_vec_shuffle_expr (tree type, tree v0, tree v1, tree mask, rtx target) >> +{ >> >> comment. >> >> +vshuffle: >> + gcc_assert (v1 == v0); >> + >> + icode = direct_optab_handler (vshuffle_optab, mode); >> >> hmm, so we don't have a vshuffle2 optab but always go via the >> builtin function, but only for constant masks there? I wonder >> if we should arrange for targets to only support a vshuffle >> optab (thus, transition away from the builtin) and so >> unconditionally have a vshuffle2 optab only (with possibly >> equivalent v1 and v0?) > > I have only implemented the case with non-constant mask that supports > only one argument. I think that it would be enough for the first > version. Later we can introduce vshuffle2 pattern and reuse the code > that expands vshuffle at the moment. Ok. >> I suppose Richard might remember what he had in mind back >> when we discussed this. >> >> Index: gcc/c-typeck.c >> === >> --- gcc/c-typeck.c (revision 177758) >> +++ gcc/c-typeck.c (working copy) >> @@ -2815,6 +2815,68 @@ build_function_call_vec (location_t loc, >> && !check_builtin_function_arguments (fundecl, nargs, argarray)) >> return error_mark_node; >> >> + /* Typecheck a builtin function which is declared with variable >> + argument list. */ >> + if (fundecl && DECL_BUILT_IN (fundecl) >> + && DECL_BUILT_IN_CLASS (fundecl) == BUILT_IN_NORMAL) >> >> just add to check_builtin_function_arguments which is called right >> in front of your added code. >> >> + /* Here we change the return type of the builtin function >> + from int f(...) --> t f(...) where t is a type of the >> + first argument. */ >> + fundecl = copy_node (fundecl); >> + TREE_TYPE (fundecl) = build_function_type (TREE_TYPE (firstarg), >> + TYPE_ARG_TYPES (TREE_TYPE >> (fundecl))); >> + function = build_fold_addr_expr (fundecl); >> >> oh, hum - now I remember ;) Eventually the C frontend should handle >> this not via the function call mechanism but similar to how Joseph >> added __builtin_complex support with >> >> 2011-08-19 Joseph Myers >> >> * c-parser.c (c_parser_postfix_expression): Handle >> RID_BUILTIN_COMPLEX. >> * doc/extend.texi (__builtin_complex): Document. >> >> and then emit VEC_SHUFFLE_EXPRs directly from the frontend. Joseph? >> >> FOR_EACH_CONSTRUCTOR_VALUE (CONSTRUCTOR_ELTS (inside_init), ix, >> value) >> - if (!CONSTANT_CLASS_P (value)) >> + if (!CONSTANT_CLASS_P (value)) >> >> watch out for spurious whitespace changes. >> >> Index: gcc/gimplify.c >> === >> --- gcc/gimplify.c (revision 177758) >> +++ gcc/gimplify.c (working copy) >> @@ -7050,6 +7050,7 @@ gimplify_expr (tree *expr_p, gimple_seq >> break; >> >> case BIT_FIELD_REF: >> + case VEC_SHUFFLE_EXPR: >> >> I don't think that's quite the right place given the is_gimple_lvalue >> predicate on the first operand. More like >> >> case VEC_SHUFFLE_EXPR: >> goto expr_3; >> >> +/* Vector shuffle expression. A = VEC_SHUFFLE_EXPR >> >> typo, mask >> >> + means >> + >> + freach i in length (mask): >> + A = mask[i] < length (v0) ? v0[mask[i]] : v1[mask[i]] >> +*/ >> +DEFTREECODE (VEC_SHUFFLE_EXPR, "vec_shuffle_expr", tcc_expression, 3) >> >> what is the (is there an
Re: [C++0x] contiguous bitfields race implementation
On Tue, Aug 30, 2011 at 8:13 PM, Aldy Hernandez wrote: > >> Btw, *byte_offset is still not relative to the containing object as >> documented, but relative to the base object of the exp reference >> tree (thus, to a in a.i.j.k.l instead of to a.i.j.k). If it were supposed >> to be relative to a.i.j.k get_inner_reference would be not needed >> either. Can you clarify what "containing object" means in the >> overall comment please? > > I'm thoroughly confused here. Originally I had "inner decl", then we > changed the nomenclature to "containing object", and now there's this > "innermost reference". Well, the nomenclature is not so important once the function only computes one variant. Only because it doesn't right now I am confused with the nomenclature trying to figure out what it is supposed to be relative to ... The containing object of a component-ref is TREE_OPERAND (exp, 0) to me. The base object would be get_base_object (exp), which is eventually what we want, right? > What I mean to say is the "a" in a.i.j.k.l. How would you like me to call > that? The innermost reference? The inner decl? Would this comment be > acceptable: > > Given a COMPONENT_REF, this function calculates the byte offset > from the innermost reference ("a" in a.i.j.k.l) to the start of the > contiguous bit region containing the field in question. from the base object ("a" in a.i.j.k.l) ... would be fine with me. >> >> If it is really relative to the innermost reference of exp you can >> "CSE" the offset of TREE_OPERAND (exp, 0) and do relative >> adjustments for all the other get_inner_reference calls. For >> example the >> >> + /* If we found the end of the bit field sequence, include the >> + padding up to the next field... */ >> if (fld) >> { >> ... >> + /* Calculate bitpos and offset of the next field. */ >> + get_inner_reference (build3 (COMPONENT_REF, >> + TREE_TYPE (exp), >> + TREE_OPERAND (exp, 0), >> + fld, NULL_TREE), >> + &tbitsize,&end_bitpos,&end_offset, >> + &tmode,&tunsignedp,&tvolatilep, true); >> >> case is not correct anyway, fld may have variable position >> (non-INTEGER_CST DECL_FIELD_OFFSET), you can't >> assume > > Innermost here means "a" in a.i.j.k.l? If so, this is what we're currently > doing, *byte_offset is the start of the bit region, and *bit_offset is the > offset from that. > > First, I thought we couldn't get a variable position here because we are now > handling that case at the beginning of the function with: > > /* Be as conservative as possible on variable offsets. */ > if (TREE_OPERAND (exp, 2) > && !host_integerp (TREE_OPERAND (exp, 2), 1)) > { > *byte_offset = TREE_OPERAND (exp, 2); > *maxbits = BITS_PER_UNIT; > *bit_offset = 0; > return; > } > > And even if we do get a variable position, I have so far being able to get > away with this... Did you test Ada and enable the C++ memory model? ;) Btw, even if the bitfield we access (and thus the whole region) is at a constant offset, the field _following_ the bitregion (the one you query above with get_inner_reference) can be at variable offset. I suggest to simply not include any padding in that case (which would be, TREE_CODE (DECL_FIELD_OFFSET (fld)) != INTEGER_CST). >> >> + *maxbits = TREE_INT_CST_LOW (maxbits_tree); >> >> this thus. > > ...because the call to fold_build2 immediately preceding this will fold away > the variable offset. You hope so ;) > Is what you want, that we call get_inner_reference once, and then use > DECL_FIELD_OFFSET+DECL_FIELD_BIT_OFFSET to calculate any subsequent bit > offset? I found this to be quite tricky with padding, and such, but am > willing to give it a whirl again. Yes. > However, could I beg you to reconsider this, and get something working > first, only later concentrating on removing the get_inner_reference() calls, > and performing any other tweaks/optimizations? Sure, it's fine to tweak this in a followup. Thanks, Richard. > Aldy >
Re: [C++0x] contiguous bitfields race implementation
On Tue, Aug 30, 2011 at 6:15 PM, Aldy Hernandez wrote: > >> *bit_offset is supposed to be relative to *byte_offset then it should >> be easy to calculate it without another get_inner_reference. > > Since, as you suggested, we will terminate early on variable length offsets, > we can assume both DECL_FIELD_OFFSET and DECL_FIELD_BIT_OFFSET will be > constants by now. Yes. > So, I assume we can calculate the bit offset like this: > > *bit_offset = (TREE_INT_CST_LOW (DECL_FIELD_OFFSET (fld)) > * BITS_PER_UNIT > + TREE_INT_CST_LOW (DECL_FIELD_BIT_OFFSET (fld))) > - (TREE_INT_CST_LOW (DECL_FIELD_OFFSET (bitregion_start)) > * BITS_PER_UNIT > + TREE_INT_CST_LOW (DECL_FIELD_BIT_OFFSET (bitregion_start))); > > (Yes, I know we can factor out the BITS_PER_UNIT and only do one > multiplication, it's just easier to read this way.) > > Is this what you had in mind? Yes. For convenience I'd simply use double_ints for the intermediate calculations. Richard.
Re: [C++0x] contiguous bitfields race implementation
On Tue, Aug 30, 2011 at 5:01 PM, Aldy Hernandez wrote: > [I'm going to respond to this piece-meal, to make sure I don't drop > anything. My apologies for the long thread, but I'm pretty sure it's in > everybody's kill file by now.] > >> + /* Be as conservative as possible on variable offsets. */ >> + if (TREE_OPERAND (exp, 2) >> +&& !host_integerp (TREE_OPERAND (exp, 2), 1)) >> + { >> + *byte_offset = TREE_OPERAND (exp, 2); >> + *maxbits = BITS_PER_UNIT; >> + return; >> + } >> >> shouldn't this be at the very beginning of the function? Because >> you've set *bit_offset to an offset that was _not_ calculated relative > > Sure. I assume in this case, *bit_offset would be 0, right? It would be DECL_FIELD_BIT_OFFSET of that field. Oh, and *byte_offset would be *byte_offset = size_binop (MULT_EXPR, TREE_OPERAND (exp, 2), size_int (DECL_OFFSET_ALIGN (field) / BITS_PER_UNIT)); see expr.c:component_ref_field_offset () (which you conveniently could use here). Note that both TREE_OPERAND (exp, 2) and compoment_ref_field_offset return offsets relative to the immediate containing struct type, not relative to the base object like get_inner_reference does ... (where it is still unclear to me what we are supposed to return from this function ...) Thus, conservative would be using get_inner_reference here, if the offset is supposed to be relative to the base object. Richard.
Re: [PATCH, PR 49886] Prevent fnsplit from changing signature when there are type attributes
On Tue, Aug 30, 2011 at 6:50 PM, Martin Jambor wrote: > Ping. Re-bootstrapped and re-tested yesterday on x86_64-linux. Ok. Does this also apply (maybe in modifed form) to the 4.6 branch? Thanks, Richard. > THanks, > > Martin > > > On Fri, Jul 29, 2011 at 10:55:31PM +0200, Martin Jambor wrote: >> Hi, >> >> On Thu, Jul 28, 2011 at 06:52:05PM +0200, Martin Jambor wrote: >> > pass_split_functions is happy to split functions which have type >> > attributes but cannot update them if the new clone has in any way >> > different parameters than the original. This can lead to >> > miscompilations in cases like the testcase. >> > >> > This patch solves it by 1) making the inliner set the >> > can_change_signature flag to false for them because their signature >> > cannot be changed (this step is also necessary to make IPA-CP operate >> > on them and handle them correctly), and 2) make the splitting pass >> > keep all parameters if the flag is set. The second step might involve >> > inventing some default definitions if the parameters did not really >> > have any. >> > >> > I spoke about this with Honza and he claimed that the new function is >> > really an entirely different thing and that the parameters may >> > correspond only very loosely and thus the type attributes should be >> > cleared. I'm not sure I agree, but in any case I needed this to work >> > to allow me continue with promised IPA-CP polishing and so I decided >> > to do this because it was easier. (My own opinion is that the current >> > representation of parameter-describing function type attributes is >> > evil and will cause harm no matter hat we do.) >> > >> >> Actually, I'd like to commit the patch below which also clears >> can_change_signature for BUILT_IN_VA_START. It is not really >> necessary for this fix but fixes some problems in a followup patch and >> is also the correct thing to do because if we clone a function calling >> it and pass non-NULL for args_to_skip, the new clone would not have a >> stdarg_p type and fold_builtin_next_arg could error when dealing with >> it. >> >> Also bootstrapped and tested on x86_64-linux. OK for trunk? >> >> Thanks, >> >> Martin >> >> >> 2011-07-29 Martin Jambor >> >> PR middle-end/49886 >> * ipa-inline-analysis.c (compute_inline_parameters): Set >> can_change_signature of noes with typde attributes. >> * ipa-split.c (split_function): Do not skip any arguments if >> can_change_signature is set. >> >> * testsuite/gcc.c-torture/execute/pr49886.c: New testcase. >> >> Index: src/gcc/ipa-inline-analysis.c >> === >> --- src.orig/gcc/ipa-inline-analysis.c >> +++ src/gcc/ipa-inline-analysis.c >> @@ -1658,18 +1658,28 @@ compute_inline_parameters (struct cgraph >> /* Can this function be inlined at all? */ >> info->inlinable = tree_inlinable_function_p (node->decl); >> >> - /* Inlinable functions always can change signature. */ >> - if (info->inlinable) >> - node->local.can_change_signature = true; >> + /* Type attributes can use parameter indices to describe them. */ >> + if (TYPE_ATTRIBUTES (TREE_TYPE (node->decl))) >> + node->local.can_change_signature = false; >> else >> { >> - /* Functions calling builtin_apply can not change signature. */ >> - for (e = node->callees; e; e = e->next_callee) >> - if (DECL_BUILT_IN (e->callee->decl) >> - && DECL_BUILT_IN_CLASS (e->callee->decl) == BUILT_IN_NORMAL >> - && DECL_FUNCTION_CODE (e->callee->decl) == BUILT_IN_APPLY_ARGS) >> - break; >> - node->local.can_change_signature = !e; >> + /* Otherwise, inlinable functions always can change signature. */ >> + if (info->inlinable) >> + node->local.can_change_signature = true; >> + else >> + { >> + /* Functions calling builtin_apply can not change signature. */ >> + for (e = node->callees; e; e = e->next_callee) >> + { >> + tree cdecl = e->callee->decl; >> + if (DECL_BUILT_IN (cdecl) >> + && DECL_BUILT_IN_CLASS (cdecl) == BUILT_IN_NORMAL >> + && (DECL_FUNCTION_CODE (cdecl) == BUILT_IN_APPLY_ARGS >> + || DECL_FUNCTION_CODE (cdecl) == BUILT_IN_VA_START)) >> + break; >> + } >> + node->local.can_change_signature = !e; >> + } >> } >> estimate_function_body_sizes (node, early); >> >> Index: src/gcc/ipa-split.c >> === >> --- src.orig/gcc/ipa-split.c >> +++ src/gcc/ipa-split.c >> @@ -945,10 +945,10 @@ static void >> split_function (struct split_point *split_point) >> { >> VEC (tree, heap) *args_to_pass = NULL; >> - bitmap args_to_skip = BITMAP_ALLOC (NULL); >> + bitmap args_to_skip; >> tree parm; >> int num = 0; >> - struct cgraph_node *node; >> + struct cgraph_node *node, *cur_node = cgraph_get_node >> (current_function_decl); >>
Re: [PATCH PING] Remove obsolete alias check in cgraph_redirect_edge_call_stmt_to_callee
On Tue, Aug 30, 2011 at 6:38 PM, Martin Jambor wrote: > > Hi, > > this is a ping of a patch that I have originally posted almost two > months ago. > > Since (same body) aliases have their own cgraph_nodes, the check for > them in cgraph_redirect_edge_call_stmt_to_callee is now unnecessary > because e->callee is now the alias, not the function node. > > The following patch therefore removes it. Bootstrapped and tested > again on x86_64-linux yesterday, OK for trunk? Ok. Thanks, Richard. > Thanks, > > Martin > > > 2011-07-08 Martin Jambor > > * cgraphunit.c (cgraph_redirect_edge_call_stmt_to_callee): Alias > check removed. > > Index: src/gcc/cgraphunit.c > === > --- src.orig/gcc/cgraphunit.c > +++ src/gcc/cgraphunit.c > @@ -2373,9 +2373,7 @@ cgraph_redirect_edge_call_stmt_to_callee > #endif > > if (e->indirect_unknown_callee > - || decl == e->callee->decl > - /* Don't update call from same body alias to the real function. */ > - || (decl && cgraph_get_node (decl) == cgraph_get_node > (e->callee->decl))) > + || decl == e->callee->decl) > return e->call_stmt; > > #ifdef ENABLE_CHECKING >