Re: [patch testsuite]: Adjust gcc.dg/tree-ssa tests for LLP64 target
2011/8/7 Mike Stump : > On Aug 6, 2011, at 6:04 AM, Kai Tietz wrote: >> this adjusts some testcases for LLP64 target x86_64 mingw. > > Ok. Please watch for any comments on stdarg... I don't have any, but others > might. Ok, will do. Applied at rev 177543. Thanks, Kai
Re: [Patch, Fortran, OOP] PR 49638: [OOP] length parameter is ignored when overriding type bound character functions with constant length.
Hi Janus, Here is a preparational patch (basically a subset of the previous one), which does not do any real changes yet, only some preparation and cleanup: * It moves check_typebound_override to interface.c and prefixes it with gfc_ (I don't like moving and modifying it at the same time). * It add the commutativity of multiplication in gfc_dep_compare_expr. * It does some minor cleanup in dependency.c (making two routines static and removing an unused argument). Ok for trunk? Cheers, Janus 2011-08-06 Janus Weil PR fortran/49638 * dependency.h (gfc_is_same_range,gfc_are_identical_variables): Remove two prototypes. * dependency.c (gfc_are_identical_variables): Made static and renamed. (gfc_dep_compare_expr): Renamed 'gfc_are_identical_variables', handle commutativity of multiplication. (gfc_is_same_range): Made static and renamed, removed argument 'def'. (check_section_vs_section): Renamed 'gfc_is_same_range'. * gfortran.h (gfc_check_typebound_override): New prototype. * interface.c (gfc_check_typebound_override): Moved here from ... * resolv.c (check_typebound_override): ... here (and renamed). (resolve_typebound_procedure): Renamed 'check_typebound_override'. OK from my side (given Mikael's comments), provided you spell resolve.c with two e :-) Regards Thomas
Re: [Patch, Fortran, OOP] PR 49638: [OOP] length parameter is ignored when overriding type bound character functions with constant length.
2011/8/7 Thomas Koenig : > Hi Janus, > >> Here is a preparational patch (basically a subset of the >> previous one), which does not do any real changes yet, only some >> preparation and cleanup: >> * It moves check_typebound_override to interface.c and prefixes it >> with gfc_ (I don't like moving and modifying it at the same time). >> * It add the commutativity of multiplication in gfc_dep_compare_expr. >> * It does some minor cleanup in dependency.c (making two routines >> static and removing an unused argument). >> >> Ok for trunk? >> >> Cheers, >> Janus >> >> >> 2011-08-06 Janus Weil >> >> PR fortran/49638 >> * dependency.h (gfc_is_same_range,gfc_are_identical_variables): >> Remove >> two prototypes. >> * dependency.c (gfc_are_identical_variables): Made static and >> renamed. >> (gfc_dep_compare_expr): Renamed 'gfc_are_identical_variables', >> handle >> commutativity of multiplication. >> (gfc_is_same_range): Made static and renamed, removed argument >> 'def'. >> (check_section_vs_section): Renamed 'gfc_is_same_range'. >> * gfortran.h (gfc_check_typebound_override): New prototype. >> * interface.c (gfc_check_typebound_override): Moved here from ... >> * resolv.c (check_typebound_override): ... here (and renamed). >> (resolve_typebound_procedure): Renamed 'check_typebound_override'. > > OK from my side (given Mikael's comments), provided you spell resolve.c with > two e :-) Thanks, guys. Committed as r177545 (including your comments). Cheers, Janus
Re: [Patch, Fortran, OOP] PR 49638: [OOP] length parameter is ignored when overriding type bound character functions with constant length.
>> OK from my side (given Mikael's comments), provided you spell resolve.c with >> two e :-) > > Thanks, guys. Committed as r177545 (including your comments). Ok, with the 'trivial' parts out of the way, the remainder of my previously proposed patch becomes rather compact (see attachment). Thomas' ongoing criticism is that rejecting all nonzero return values of 'gfc_dep_compare_expr' will reject too much (i.e. cases where we can not prove that the expressions are equal, but they may still be). My feeling is that only throwing a warning for a result of '-2' is too weak, because the majority of cases will have that result (just think 'len=3' vs. 'len=n'). Instead I could offer to try to extend the return values of 'gfc_dep_compare_expr' to distinguish between the following cases (which right now would both result in '-2'): a) We can not determine the relationship between both expressions, but we know they are different for certain input values. (This case would include e.g. different expr_type) b) We cannot make any statement. Is this the way to go? Or are there any other proposals for resolving this argument? Cheers, Janus Index: gcc/fortran/interface.c === --- gcc/fortran/interface.c (revision 177545) +++ gcc/fortran/interface.c (working copy) @@ -3556,15 +3556,25 @@ gfc_check_typebound_override (gfc_symtree* proc, g } /* FIXME: Do more comprehensive checking (including, for instance, the - rank and array-shape). */ + array-shape). */ gcc_assert (proc_target->result && old_target->result); - if (!gfc_compare_types (&proc_target->result->ts, - &old_target->result->ts)) + if (!compare_type_rank (proc_target->result, old_target->result)) { gfc_error ("'%s' at %L and the overridden FUNCTION should have" - " matching result types", proc->name, &where); + " matching result types and ranks", proc->name, &where); return FAILURE; } + + /* Check string length. */ + if (proc_target->result->ts.type == BT_CHARACTER + && proc_target->result->ts.u.cl && old_target->result->ts.u.cl + && gfc_dep_compare_expr (proc_target->result->ts.u.cl->length, + old_target->result->ts.u.cl->length) != 0) + { + gfc_error ("Character length mismatch between '%s' at '%L' " + "and overridden FUNCTION", proc->name, &where); + return FAILURE; + } } /* If the overridden binding is PUBLIC, the overriding one must not be Index: gcc/fortran/dependency.c === --- gcc/fortran/dependency.c (revision 177545) +++ gcc/fortran/dependency.c (working copy) @@ -123,8 +123,18 @@ are_identical_variables (gfc_expr *e1, gfc_expr *e { gfc_ref *r1, *r2; - if (e1->symtree->n.sym != e2->symtree->n.sym) -return false; + if (e1->symtree->n.sym->attr.dummy && e2->symtree->n.sym->attr.dummy) +{ + /* Dummy arguments: Only check for equal names. */ + if (e1->symtree->n.sym->name != e2->symtree->n.sym->name) + return false; +} + else +{ + /* Check for equal symbols. */ + if (e1->symtree->n.sym != e2->symtree->n.sym) + return false; +} /* Volatile variables should never compare equal to themselves. */
Re: [PATCH] [JAVA] Double.parseDouble(null) throw NullPointerException
Hi, When I use gcj on an RTOS(RTEMS), Double.parseDouble(null) throw NumberFormatException, but it should throw NullPointerException. So I add the patch below: Index: natVMDouble.cc === --- natVMDouble.cc (revision 172224) +++ natVMDouble.cc (working copy) @@ -19,6 +19,7 @@ #include #include #include +#include #include #include @@ -162,6 +163,9 @@ jdouble java::lang::VMDouble::parseDouble(jstring str) { + if(str == NULL) + throw new NullPointerException(); + int length = str->length(); while (length > 0 The testsuite/Throw_2.java has been PASS after this patch. what do you think about this patch? Thanks, Jie
Re: [PATCH PR43513, 1/3] Replace vla with array - Implementation.
On 07/30/2011 09:21 AM, Tom de Vries wrote: > Hi, > > On 07/28/2011 08:20 PM, Tom de Vries wrote: >> On 07/28/2011 06:25 PM, Richard Guenther wrote: >>> On Thu, 28 Jul 2011, Tom de Vries wrote: >>> On 07/28/2011 12:22 PM, Richard Guenther wrote: > On Wed, 27 Jul 2011, Tom de Vries wrote: > >> On 07/27/2011 05:27 PM, Richard Guenther wrote: >>> On Wed, 27 Jul 2011, Tom de Vries wrote: >>> On 07/27/2011 02:12 PM, Richard Guenther wrote: > On Wed, 27 Jul 2011, Tom de Vries wrote: > >> On 07/27/2011 01:50 PM, Tom de Vries wrote: >>> Hi Richard, >>> >>> I have a patch set for bug 43513 - The stack pointer is adjusted >>> twice. >>> >>> 01_pr43513.3.patch >>> 02_pr43513.3.test.patch >>> 03_pr43513.3.mudflap.patch >>> >>> The patch set has been bootstrapped and reg-tested on x86_64. >>> >>> I will sent out the patches individually. >>> >> >> The patch replaces a vla __builtin_alloca that has a constant >> argument with an >> array declaration. >> >> OK for trunk? > > I don't think it is safe to try to get at the VLA type the way you do. I don't understand in what way it's not safe. Do you mean I don't manage to find the type always, or that I find the wrong type, or something else? >>> >>> I think you might get the wrong type, >> >> Ok, I'll review that code one more time. >> >>> you also do not transform code >>> like >>> >>> int *p = alloca(4); >>> *p = 3; >>> >>> as there is no array type involved here. >>> >> >> I was trying to stay away from non-vla allocas. A source declared >> alloca has >> function livetime, so we could have a single alloca in a loop, called 10 >> times, >> with all 10 instances live at the same time. This patch does not detect >> such >> cases, and thus stays away from non-vla allocas. A vla decl does not >> have such >> problems, the lifetime ends when it goes out of scope. > > Yes indeed - that probably would require more detailed analysis. > > In fact I would simply do sth like > > elem_type = build_nonstandard_integer_type (BITS_PER_UNIT, 1); > n_elem = size * 8 / BITS_PER_UNIT; > array_type = build_array_type_nelts (elem_type, n_elem); > var = create_tmp_var (array_type, NULL); > return fold_convert (TREE_TYPE (lhs), build_fold_addr_expr (var)); > I tried this code on the example, and it works, but the newly declared type has an 8-bit alignment, while the vla base type has a 32 bit alignment. This make the memory access in the example potentially unaligned, which prohibits an ivopts optimization, so the resulting text size is 68 instead of the 64 achieved with my current patch. >>> >>> Ok, so then set DECL_ALIGN of the variable to something reasonable >>> like MIN (size * 8, GET_MODE_PRECISION (word_mode)). Basically the >>> alignment that the targets alloca function would guarantee. >>> >> >> I tried that, but that doesn't help. It's the alignment of the type that >> matters, not of the decl. > > It shouldn't. All accesses are performed with the original types and > alignment comes from that (plus the underlying decl). > I managed to get it all working by using build_aligned_type rather that DECL_ALIGN. >>> >>> That's really odd, DECL_ALIGN should just work - nothing refers to the >>> type of the decl in the IL. Can you try also setting DECL_USER_ALIGN to >>> 1 maybe? >>> >> >> This doesn't work either. >> >> /* Declare array. */ >> elem_type = build_nonstandard_integer_type (BITS_PER_UNIT, 1); >> n_elem = size * 8 / BITS_PER_UNIT; >> align = MIN (size * 8, GET_MODE_PRECISION (word_mode)); >> array_type = build_array_type_nelts (elem_type, n_elem); >> var = create_tmp_var (array_type, NULL); >> DECL_ALIGN (var) = align; >> DECL_USER_ALIGN (var) = 1; >> >> Maybe this clarifies it: >> >> Breakpoint 1, may_be_unaligned_p (ref=0xf7d9d410, step=0xf7d3d578) at >> /home/vries/local/google/src/gcc-mainline/gcc/tree-ssa-loop-ivopts.c:1621 >> (gdb) call debug_generic_expr (ref) >> MEM[(int[0:D.2579] *)&D.2595][0] >> (gdb) call debug_generic_expr (step) >> 4 >> >> 1627 base = get_inner_reference (ref, &bitsize, &bitpos, &toffset, &mode, >> (gdb) call debug_generic_expr (base) >> D.2595 >> >> 1629 base_type = TREE_TYPE (base); >> (gdb) call debug_generic_expr (base_type) >> [40] >> >> 1630 base_align = TYPE_ALIGN (base_type); >> (gdb) p base_align >> $1 = 8 >> >> So the align is 8-bits, and we return true here: >> >> (gdb) n >> 1632 if (mode != BLKmode) >> (gdb) n
Re: [v3] doxygen markup fixes
On 7 August 2011 08:30, Benjamin Kosnik wrote: > > This fixes markup that gives warnings or errors as part of html/pdf > documentation generation. Most of this stuff is pretty simple, and is > just making sure that the documented parameter names exactly match the > signatures. > > tested x86/linux > > -benjamin +/** @file bits/alloc_traits.h + * This is an internal header file, included by other library headers. + * Do not attempt to use it directly. @headername{scoped_allocator} + */ + The correct header for allocator_traits is
Re: [Patch, Fortran, OOP] PR 49638: [OOP] length parameter is ignored when overriding type bound character functions with constant length.
Am 07.08.2011 12:56, schrieb Janus Weil: + /* Check string length. */ + if (proc_target->result->ts.type == BT_CHARACTER + && proc_target->result->ts.u.cl&& old_target->result->ts.u.cl + && gfc_dep_compare_expr (proc_target->result->ts.u.cl->length, + old_target->result->ts.u.cl->length) != 0) + { + gfc_error ("Character length mismatch between '%s' at '%L' " +"and overridden FUNCTION", proc->name,&where); + return FAILURE; + } } Well, let's make this into (again, typing the patch directly into e-mail) /* Check string length. */ if (proc_target->result->ts.type == BT_CHARACTER && proc_target->result->ts.u.cl && old_target->result->ts.u.cl { int compval = gfc_dep_compare_expr (proc_target->result->ts.u.cl->length, old_target->result->ts.u.cl->length); switch (compval) { case -3: case -1: case 1: gfc_error ("Character length mismatch between '%s' at '%L' " "and overridden FUNCTION", proc->name, &where); return FAILURE; case -2: gfc_warning ("Possible length mismatch between '%s' at '%L' " "and overriden FUNCTION, proc->name, &where); break; case 0: break; default: gfc_internal_error ("Unexpected return of gfc_dep_compare_expr"; break; } and then work on extending gfc_dep_compare_expr to return -3 for more cases. I can help with that. Regards Thomas
Re: [v3] doxygen markup fixes
On 7 August 2011 12:53, Jonathan Wakely wrote: > On 7 August 2011 08:30, Benjamin Kosnik wrote: >> >> This fixes markup that gives warnings or errors as part of html/pdf >> documentation generation. Most of this stuff is pretty simple, and is >> just making sure that the documented parameter names exactly match the >> signatures. >> >> tested x86/linux >> >> -benjamin > > +/** @file bits/alloc_traits.h > + * This is an internal header file, included by other library headers. > + * Do not attempt to use it directly. @headername{scoped_allocator} > + */ > + > > The correct header for allocator_traits is > Fixed like so. 2011-08-07 Jonathan Wakely * include/bits/alloc_traits.h: Fix doxygen @headername. Index: include/bits/alloc_traits.h === --- include/bits/alloc_traits.h (revision 177545) +++ include/bits/alloc_traits.h (working copy) @@ -24,7 +24,7 @@ /** @file bits/alloc_traits.h * This is an internal header file, included by other library headers. - * Do not attempt to use it directly. @headername{scoped_allocator} + * Do not attempt to use it directly. @headername{memory} */ #ifndef _ALLOC_TRAITS_H
Re: [RFC PATCH, i386]: Allow zero_extended addresses (+ problems with reload and offsetable address, "o" constraint)
On Fri, Aug 5, 2011 at 8:51 PM, Uros Bizjak wrote: > As I read this sentence, the RTX is forced into a temporary register, > and reload tries to satisfy "o" constraint with plus ((reg ...) > (const_int ...)), as said at the introduction of "o" constraint a > couple of pages earlier. Unfortunately, this does not seem to be the > case. > > Is there anything wrong with my approach, or is there something wrong in > reload? To answer my own question, the problem was in *add3_doubleword pattern, defined as: (define_insn_and_split "*add3_doubleword" [(set (match_operand: 0 "nonimmediate_operand" "=r,o") (plus: (match_operand: 1 "nonimmediate_operand" "%0,0") (match_operand: 2 "" "ro,r"))) (clobber (reg:CC FLAGS_REG))] "ix86_binary_operator_ok (PLUS, mode, operands)" When reload tried to satisfy alternative 1 (the "o" and matching "0") with a non-offsettable (in this particular case, zero-extended) address, it CSE'd operand 0 and operand 1 to a temporary TImode register. Unfortunately a Timode move has its own constraints: (define_insn "*movti_internal_rex64" [(set (match_operand:TI 0 "nonimmediate_operand" "=!r,o,x,x,xm") (match_operand:TI 1 "general_operand" "riFo,riF,C,xm,x"))] "TARGET_64BIT && !(MEM_P (operands[0]) && MEM_P (operands[1]))" where move from/to a general register to/from non-offsettable memory is not valid. Although, it would be nice for reload to subsequently fix CSE'd non-offsetable memory by copying address to temporary reg (*as said in the documentation*), we could simply require an XMM temporary for TImode reloads to/from integer registers, and this fixes ICE for x32. The testcase to play with (gcc -O2 -mx32): --cut here-- void test (__int128 *array, int idx, int off) { __int128 *dest = &array [idx]; dest[0] += 1; dest[off] = 0; } --cut here-- So, following additional patch saves the day: Index: i386/i386.c === --- i386/i386.c (revision 177536) +++ i386/i386.c (working copy) @@ -28233,6 +28248,15 @@ ix86_secondary_reload (bool in_p, rtx x, reg_class enum machine_mode mode, secondary_reload_info *sri ATTRIBUTE_UNUSED) { + /* Double-word spills from general registers to non-offsettable + memory references go through XMM register. Following code + handles zero-extended addresses on x32 target. */ + if (TARGET_64BIT + && GET_MODE_SIZE (mode) > UNITS_PER_WORD + && rclass == GENERAL_REGS + && !offsettable_memref_p (x)) +return SSE_REGS; + /* QImode spills from non-QI registers require intermediate register on 32bit targets. */ if (!TARGET_64BIT Uros.
Re: [Patch, Fortran, OOP] PR 49638: [OOP] length parameter is ignored when overriding type bound character functions with constant length.
2011/8/7 Thomas Koenig : > Am 07.08.2011 12:56, schrieb Janus Weil: >> >> + /* Check string length. */ >> + if (proc_target->result->ts.type == BT_CHARACTER >> + && proc_target->result->ts.u.cl&& old_target->result->ts.u.cl >> + && gfc_dep_compare_expr (proc_target->result->ts.u.cl->length, >> + old_target->result->ts.u.cl->length) != >> 0) >> + { >> + gfc_error ("Character length mismatch between '%s' at '%L' " >> + "and overridden FUNCTION", proc->name,&where); >> + return FAILURE; >> + } >> } > > Well, let's make this into (again, typing the patch directly into > e-mail) > > [...] > > and then work on extending gfc_dep_compare_expr to return -3 for more cases. > I can help with that. Alright then. How about this: I'll commit the attached verision of the patch (including your suggestions), and we start messing with the return values afterwards? Patch is regtested on x86_64-unknown-linux-gnu. I hope the test case is sufficient for a start. Cheers, Janus 2011-08-07 Janus Weil PR fortran/49638 * dependency.c (are_identical_variables): For dummy arguments only check for equal names, not equal symbols. * interface.c (gfc_check_typebound_override): Add checking for rank and character length. 2011-08-07 Janus Weil PR fortran/49638 * gfortran.dg/typebound_override_1.f90: New. Index: gcc/fortran/interface.c === --- gcc/fortran/interface.c (revision 177545) +++ gcc/fortran/interface.c (working copy) @@ -3556,15 +3556,43 @@ gfc_check_typebound_override (gfc_symtree* proc, g } /* FIXME: Do more comprehensive checking (including, for instance, the - rank and array-shape). */ + array-shape). */ gcc_assert (proc_target->result && old_target->result); - if (!gfc_compare_types (&proc_target->result->ts, - &old_target->result->ts)) + if (!compare_type_rank (proc_target->result, old_target->result)) { gfc_error ("'%s' at %L and the overridden FUNCTION should have" - " matching result types", proc->name, &where); + " matching result types and ranks", proc->name, &where); return FAILURE; } + + /* Check string length. */ + if (proc_target->result->ts.type == BT_CHARACTER + && proc_target->result->ts.u.cl && old_target->result->ts.u.cl) + { + int compval = gfc_dep_compare_expr (proc_target->result->ts.u.cl->length, + old_target->result->ts.u.cl->length); + switch (compval) + { + case -1: + case 1: + gfc_error ("Character length mismatch between '%s' at '%L' and " + "overridden FUNCTION", proc->name, &where); + return FAILURE; + + case -2: + gfc_warning ("Possible character length mismatch between '%s' at" + " '%L' and overridden FUNCTION", proc->name, &where); + break; + + case 0: + break; + + default: + gfc_internal_error ("gfc_check_typebound_override: Unexpected " + "result %i of gfc_dep_compare_expr", compval); + break; + } + } } /* If the overridden binding is PUBLIC, the overriding one must not be Index: gcc/fortran/dependency.c === --- gcc/fortran/dependency.c (revision 177545) +++ gcc/fortran/dependency.c (working copy) @@ -123,8 +123,18 @@ are_identical_variables (gfc_expr *e1, gfc_expr *e { gfc_ref *r1, *r2; - if (e1->symtree->n.sym != e2->symtree->n.sym) -return false; + if (e1->symtree->n.sym->attr.dummy && e2->symtree->n.sym->attr.dummy) +{ + /* Dummy arguments: Only check for equal names. */ + if (e1->symtree->n.sym->name != e2->symtree->n.sym->name) + return false; +} + else +{ + /* Check for equal symbols. */ + if (e1->symtree->n.sym != e2->symtree->n.sym) + return false; +} /* Volatile variables should never compare equal to themselves. */ ! { dg-do compile } ! ! PR 49638: [OOP] length parameter is ignored when overriding type bound character functions with constant length. ! ! Original test case contributed by Hans-Werner Boschmann module m implicit none type :: t1 contains procedure, nopass :: a => a1 procedure, nopass :: b => b1 procedure, nopass :: c => c1 procedure, nopass :: d => d1 procedure, nopass :: e => e1 end type type, extends(t1) :: t2 contains procedure, nopass :: a => a2 ! { dg-error "Character length mismatch" } procedure, nopass :: b => b2 ! { dg-error "should have matching result types and ranks" } procedure, nopass :: c => c2 ! { dg-warning "Possible character length mismatch" } procedure, nopass :: d => d2 ! valid, check for commutativity (+,*) procedure, nopass :: e => e2 ! { dg-warning "Possible character length mismatch" } end type contains function a1 () characte
Re: [PATCH] ARM fixed-point support [5.5/6]: argument & return padding for libcalls
Hi Julian, Julian Brown writes: > This patch allows padding to be specified per-target for libcalls. This > hasn't been traditionally important, because libcalls haven't accepted > quantities which might need padding, but that's no longer true with the > new(-ish) fixed-point support helper functions. > > Tested (alongside other fixed-point support patches) with cross to ARM > EABI in both big & little-endian mode (the target-specific part is to > avoid a behaviour change for half-float types on ARM). OK to apply? This patch caused several regressions on big-endian 64-bit MIPS targets, which now try to shift single-precision floating-point arguments to the top of an FPR. The calls.c part... > diff --git a/gcc/calls.c b/gcc/calls.c > index 44a16ff..9d5d294 100644 > --- a/gcc/calls.c > +++ b/gcc/calls.c > @@ -3794,13 +3794,41 @@ emit_library_call_value_1 (int retval, rtx orgfun, > rtx value, >rtx val = argvec[argnum].value; >rtx reg = argvec[argnum].reg; >int partial = argvec[argnum].partial; > - > + int size = 0; > + >/* Handle calls that pass values in multiple non-contiguous >locations. The PA64 has examples of this for library calls. */ >if (reg != 0 && GET_CODE (reg) == PARALLEL) > emit_group_load (reg, val, NULL_TREE, GET_MODE_SIZE (mode)); >else if (reg != 0 && partial == 0) > - emit_move_insn (reg, val); > +{ > + emit_move_insn (reg, val); > +#ifdef BLOCK_REG_PADDING > + size = GET_MODE_SIZE (argvec[argnum].mode); > + > + /* Copied from load_register_parameters. */ > + > + /* Handle case where we have a value that needs shifting > + up to the msb. eg. a QImode value and we're padding > + upward on a BYTES_BIG_ENDIAN machine. */ > + if (size < UNITS_PER_WORD > + && (argvec[argnum].locate.where_pad > + == (BYTES_BIG_ENDIAN ? upward : downward))) > + { > + rtx x; > + int shift = (UNITS_PER_WORD - size) * BITS_PER_UNIT; > + > + /* Assigning REG here rather than a temp makes CALL_FUSAGE > + report the whole reg as used. Strictly speaking, the > + call only uses SIZE bytes at the msb end, but it doesn't > + seem worth generating rtl to say that. */ > + reg = gen_rtx_REG (word_mode, REGNO (reg)); > + x = expand_shift (LSHIFT_EXPR, word_mode, reg, shift, reg, 1); > + if (x != reg) > + emit_move_insn (reg, x); > + } > +#endif > + } > >NO_DEFER_POP; > } ...looks good in itself. The problem on MIPS is the same one that you mention in this "???" comment: > diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c > index 7d52b0e..cd32fe3 100644 > --- a/gcc/config/arm/arm.c > +++ b/gcc/config/arm/arm.c > @@ -11375,6 +11375,15 @@ arm_pad_arg_upward (enum machine_mode mode, > const_tree type) >if (type && BYTES_BIG_ENDIAN && INTEGRAL_TYPE_P (type)) > return false; > > + /* Half-float values are only passed to libcalls, not regular functions. > + They should be passed and returned as "short"s (see RTABI). To achieve > + that effect in big-endian mode, pad downwards so the value is passed in > + the least-significant end of the register. ??? This needs to be here > + rather than in arm_pad_reg_upward due to peculiarity in the handling of > + libcall arguments. */ > + if (BYTES_BIG_ENDIAN && mode == HFmode) > +return false; > + in that the value of argvec[argnum].locate.where_pad is coming from the wrong macro: FUNCTION_ARG_PADDING rather than BLOCK_REG_PADDING. So while the code above is conditional on BLOCK_REG_PADDING, it's actually using the value returned by FUNCTION_ARG_PADDING instead. This represents an ABI change. It looks like your arm.c patch is trying to partially undo that change for ARM, but it still affects other targets in a similar way. The patch below borrows the padding code from the main call routines. It fixes the MIPS problems for me (tested on mips64-linux-gnu), but I'm not set up for big-endian ARM testing. From what I can tell, other targets' BLOCK_REG_PADDING definitions already handle null types. Richard Index: gcc/calls.c === *** gcc/calls.c 2011-08-07 11:11:23.0 +0100 --- gcc/calls.c 2011-08-07 11:11:27.0 +0100 *** emit_library_call_value_1 (int retval, r *** 3576,3595 argvec[count].partial = targetm.calls.arg_partial_bytes (args_so_far, mode, NULL_TREE, 1); ! locate_and_pad_parm (mode, NULL_TREE, #ifdef STACK_PARMS_IN_REG_PARM_AREA ! 1, #else ! argvec[count].reg != 0, #endif - argvec[count].partial, - NULL_TREE, &args_size, &argvec[count].locate); - - gcc_assert (!argvec[count].locate.size.var); -
Re: [Patch, Fortran, OOP] PR 49638: [OOP] length parameter is ignored when overriding type bound character functions with constant length.
2011/8/7 Janus Weil : > 2011/8/7 Thomas Koenig : >> Am 07.08.2011 12:56, schrieb Janus Weil: >>> >>> + /* Check string length. */ >>> + if (proc_target->result->ts.type == BT_CHARACTER >>> + && proc_target->result->ts.u.cl&& old_target->result->ts.u.cl >>> + && gfc_dep_compare_expr (proc_target->result->ts.u.cl->length, >>> + old_target->result->ts.u.cl->length) != >>> 0) >>> + { >>> + gfc_error ("Character length mismatch between '%s' at '%L' " >>> + "and overridden FUNCTION", proc->name,&where); >>> + return FAILURE; >>> + } >>> } >> >> Well, let's make this into (again, typing the patch directly into >> e-mail) >> >> [...] >> >> and then work on extending gfc_dep_compare_expr to return -3 for more cases. >> I can help with that. > > Alright then. How about this: I'll commit the attached verision of the > patch (including your suggestions), and we start messing with the > return values afterwards? Patch is regtested on > x86_64-unknown-linux-gnu. I hope the test case is sufficient for a > start. > > Cheers, > Janus > > > 2011-08-07 Janus Weil Sorry, completely forgot to mention Thomas in the ChangeLog. Of course this should be 2011-08-07 Janus Weil Thomas Koenig Cheers, Janus > > PR fortran/49638 > * dependency.c (are_identical_variables): For dummy arguments only > check for equal names, not equal symbols. > * interface.c (gfc_check_typebound_override): Add checking for rank > and character length. > > 2011-08-07 Janus Weil > > PR fortran/49638 > * gfortran.dg/typebound_override_1.f90: New. >
Re: [Patch, Fortran, OOP] PR 49638: [OOP] length parameter is ignored when overriding type bound character functions with constant length.
Am 07.08.2011 19:05, schrieb Janus Weil: 2011/8/7 Thomas Koenig: Am 07.08.2011 12:56, schrieb Janus Weil: + /* Check string length. */ + if (proc_target->result->ts.type == BT_CHARACTER +&&proc_target->result->ts.u.cl&&old_target->result->ts.u.cl +&&gfc_dep_compare_expr (proc_target->result->ts.u.cl->length, + old_target->result->ts.u.cl->length) != 0) + { + gfc_error ("Character length mismatch between '%s' at '%L' " +"and overridden FUNCTION", proc->name,&where); + return FAILURE; + } } Well, let's make this into (again, typing the patch directly into e-mail) [...] and then work on extending gfc_dep_compare_expr to return -3 for more cases. I can help with that. Alright then. How about this: I'll commit the attached verision of the patch (including your suggestions), and we start messing with the return values afterwards? Patch is regtested on x86_64-unknown-linux-gnu. I hope the test case is sufficient for a start. This is OK. Thanks for the patch (and for bearing with me ;-) When extending the values of gfc_dep_compare_expr, we will need to go through all its uses (making sure we change == -2 to <= -2). Regards Thomas
PATCH: PR middle-end/49721: convert_memory_address_addr_space may generate invalid new insns
Hi, We transform ptr_extend:DI (plus:SI (FOO:SI) (const_int Y))) to (plus:DI (ptr_extend:DI (FOO:SI)) (const_int Y)) since this is how Pmode != ptr_mode is supported even if the resulting address may overflow/underflow. It is also true for x32 which has zero_extend instead of ptr_extend. I have tried different approaches to avoid transforming (zero_extend:DI (plus:SI (FOO:SI) (const_int Y))) to (plus:DI (zero_extend:DI (FOO:SI)) (const_int Y)) without success. This patch relaxes the condition to check POINTERS_EXTEND_UNSIGNED != 0 instead if POINTERS_EXTEND_UNSIGNED < 0 to cover both ptr_extend and zero_extend. We can investigate a better approach for ptr_extend and zero_extend later. For now, I believe it is the saftest way to support ptr_extend and zero_extend. Any comments? Thanks. H.J. --- gcc/ 2011-08-06 H.J. Lu PR middle-end/49721 * explow.c (convert_memory_address_addr_space): Also permute the conversion and addition of constant for zero-extend. gcc/testsuite/ 2011-08-06 H.J. Lu PR middle-end/49721 * gfortran.dg/pr49721.f: New. diff --git a/gcc/explow.c b/gcc/explow.c index f8262db..ed2f621 100644 --- a/gcc/explow.c +++ b/gcc/explow.c @@ -384,18 +384,23 @@ convert_memory_address_addr_space (enum machine_mode to_mode ATTRIBUTE_UNUSED, case PLUS: case MULT: - /* For addition we can safely permute the conversion and addition -operation if one operand is a constant and converting the constant -does not change it or if one operand is a constant and we are -using a ptr_extend instruction (POINTERS_EXTEND_UNSIGNED < 0). + /* FIXME: For addition, we used to permute the conversion and + * addition operation only if one operand is a constant and +converting the constant does not change it or if one operand +is a constant and we are using a ptr_extend instruction +(POINTERS_EXTEND_UNSIGNED < 0) even if the resulting address +may overflow/underflow. We relax the condition to include +zero-extend (POINTERS_EXTEND_UNSIGNED > 0) since the other +parts of the compiler depend on it. See PR 49721. + We can always safely permute them if we are making the address narrower. */ if (GET_MODE_SIZE (to_mode) < GET_MODE_SIZE (from_mode) || (GET_CODE (x) == PLUS && CONST_INT_P (XEXP (x, 1)) - && (XEXP (x, 1) == convert_memory_address_addr_space - (to_mode, XEXP (x, 1), as) - || POINTERS_EXTEND_UNSIGNED < 0))) + && (POINTERS_EXTEND_UNSIGNED != 0 + || XEXP (x, 1) == convert_memory_address_addr_space + (to_mode, XEXP (x, 1), as return gen_rtx_fmt_ee (GET_CODE (x), to_mode, convert_memory_address_addr_space (to_mode, XEXP (x, 0), as), diff --git a/gcc/testsuite/gfortran.dg/pr49721.f b/gcc/testsuite/gfortran.dg/pr49721.f new file mode 100644 index 000..39e2ed7 --- /dev/null +++ b/gcc/testsuite/gfortran.dg/pr49721.f @@ -0,0 +1,35 @@ +! PR middle-end/49721 +! { dg-do compile } +! { dg-options "-O3 -funroll-loops" } + + subroutine midbloc6(c,a2,a2i,q) + parameter (ndim2=6) + parameter (ndim=3) + dimension ri(ndim2),cr(ndim2,ndim2),xj(ndim2,ndim2),q(*) + @,sai(ndim2,ndim2),cm(ndim2,ndim2),w(ndim2,ndim2) + dimension vr(ndim2,ndim2),vi(ndim2,ndim2),s1(ndim2,ndim2),p(ndim) + dimension xq(6),qb(2),qc(2),ifl(6),iplane(3) + save + call eig66(cr,rr,ri,vr,vi) + xq(i)=asin(ri(i))/x2pi + i9=6 + qb(1)=q(1)/x2pi +do 180 i=1,2 + do 170 j=1,6 + 120 if(xq(j)) 130,190,140 + 130 if(qb(i)-0.5d0) 160,150,150 + 140 if(qb(i)-0.5d0) 150,150,160 + 150 continue +tst=abs(abs(qb(i))-abs(xq(j))) + 160 continue + 170 continue + iplane(i)=k + 180 continue + 190 continue + n1=iplane(3) + if(i9.eq.6) then +z=vr(1,n1)*vi(2,n1)-vr(2,n1)*vi(1,n1)+vr(3,n1)*vi(4,n1)-vr(4,n1) + endif + sai(6,i)=vi(i,n1)/z + call dacond6(a2,zero) + end
Re: Support -mcpu=native on Solaris/SPARC
> 2011-07-27 Rainer Orth > > gcc: > * config/sparc/driver-sparc.c: New file. > * config/sparc/x-sparc: New file. > * config.host: Use driver-sparc.o, sparc/x-sparc on > sparc*-*-solaris2*. > * config/sparc/sparc.opt (native): New value for enum > processor_type. > * config/sparc/sparc-opts.h (PROCESSOR_NATIVE): Declare. > * config/sparc/sparc.c (sparc_option_override): Abort if > PROCESSOR_NATIVE gets here. > * config/sparc/sol2.h [__sparc__] (host_detect_local_cpu): Declare. > (EXTRA_SPEC_FUNCTIONS, MCPU_MTUNE_NATIVE_SPECS, > DRIVER_SELF_SPECS): Define. > * configure.ac (EXTRA_GCC_LIBS): Check for libkstat. > Substitute result. > * configure: Regenerate. > * Makefile.in (EXTRA_GCC_LIBS): Set. > (xgcc$(exeext)): Add $(EXTRA_GCC_LIBS). > (cpp$(exeext)): Likewise. OK if you document the new value in doc/invoke.texi, thanks in advance. -- Eric Botcazou
PATCH: Add -mavx2 and properly check numbers of mask bits
Hi, opth-gen.awk has print "#define " mask name " (1 << " masknum[vname]++ ")" and int has 32bits. We should check if (masknum[var] > 32) instead of if (masknum[var] > 31) Now, I got #define OPTION_MASK_ISA_X32 (1 << 30) #define OPTION_MASK_ISA_XOP (1 << 31) in options.h. OK for trunk? Thanks. H.J. --- 2011-08-07 H.J. Lu * opth-gen.awk: Properly check numbers of mask bits. * config/i386/i386.opt: Add mavx2. diff --git a/gcc/config/i386/i386.opt b/gcc/config/i386/i386.opt index f197dd8..3cfc812 100644 --- a/gcc/config/i386/i386.opt +++ b/gcc/config/i386/i386.opt @@ -457,6 +457,10 @@ mavx Target Report Mask(ISA_AVX) Var(ix86_isa_flags) Save Support MMX, SSE, SSE2, SSE3, SSSE3, SSE4.1, SSE4.2 and AVX built-in functions and code generation +mavx2 +Target Report Mask(ISA_AVX2) Var(ix86_isa_flags) Save +Support MMX, SSE, SSE2, SSE3, SSSE3, SSE4.1, SSE4.2, AVX and AVX2 built-in functions and code generation + mfma Target Report Mask(ISA_FMA) Var(ix86_isa_flags) Save Support MMX, SSE, SSE2, SSE3, SSSE3, SSE4.1, SSE4.2, AVX and FMA built-in functions and code generation diff --git a/gcc/opth-gen.awk b/gcc/opth-gen.awk index 876e0f6..5d156f5 100644 --- a/gcc/opth-gen.awk +++ b/gcc/opth-gen.awk @@ -311,7 +311,7 @@ for (i = 0; i < n_extra_masks; i++) { } for (var in masknum) { - if (masknum[var] > 31) { + if (masknum[var] > 32) { if (var == "") print "#error too many target masks" else
Re: [Patch, Fortran, OOP] PR 49638: [OOP] length parameter is ignored when overriding type bound character functions with constant length.
>> Alright then. How about this: I'll commit the attached verision of the >> patch (including your suggestions), and we start messing with the >> return values afterwards? Patch is regtested on >> x86_64-unknown-linux-gnu. I hope the test case is sufficient for a >> start. > > This is OK. Thanks for the patch (and for bearing with me ;-) Well, thanks for the thorough discussion :) Committed as r177550. > When extending the values of gfc_dep_compare_expr, we will need to go > through all its uses (making sure we change == -2 to <= -2). Right. I will try to take care of that during the coming week (unless you beat me to it). Cheers, Janus
Re: [PATCH] Fix PR49518
On Tue, Jul 5, 2011 at 2:35 AM, Richard Guenther wrote: > On Tue, 5 Jul 2011, Ira Rosen wrote: > >> Richard Guenther wrote on 04/07/2011 03:30:59 PM: >> > > >> > > Richard Guenther wrote on 04/07/2011 02:38:50 PM: >> > > >> > > > Handling of negative steps broke one of the many asserts in >> > > > the vectorizer. The following patch drops one that I can't >> > > > make sense of. I think all asserts need comments - especially >> > > > this one would, as I can't see why using vf is correct to >> > > > test against and not nelements (and why <= vf and not < vf). >> > > >> > > There is an explanation 10 rows above the assert. It doesn't make sense >> to >> > > peel more than vf iterations (and not nelements, since for the case of >> > > multiple types it may help to align more data-refs - see the comment in >> the >> > > code). IIRC <= is for the case of aligned access, but I am not sure >> about >> > > that, so maybe you are right. >> > > >> > > I don't see how it is related to negative steps. >> > > >> > > I think that the real reason for this failure is that the loads are >> > > actually irrelevant (hence, vf=4 that doesn't take char loads into >> > > account), but we don't check that when we analyze data-refs. So, in my >> > > opinion, the proper fix will add such check. >> > >> > The following also works for me: >> > >> > Index: tree-vect-data-refs.c >> > === >> > --- tree-vect-data-refs.c (revision 175802) >> > +++ tree-vect-data-refs.c (working copy) >> > @@ -1495,6 +1495,9 @@ vect_enhance_data_refs_alignment (loop_v >> > stmt = DR_STMT (dr); >> > stmt_info = vinfo_for_stmt (stmt); >> > >> > + if (!STMT_VINFO_RELEVANT (stmt_info)) >> > + continue; >> > + >> > /* For interleaving, only the alignment of the first access >> > matters. */ >> > if (STMT_VINFO_STRIDED_ACCESS (stmt_info) >> > >> > does that look better or do you propose to clean the datarefs >> > vector from those references? >> >> Well, this is certainly enough to fix the PR. >> I am not sure if we can just remove these data-refs from the dependence >> checks. After that all the alignment and access checks are at least >> redundant. > > Ok. The following is what I have tested for this PR and also for > PR49628. > > Bootstrapped and tested on x86_64-unknown-linux-gnu, ok for trunk? > > Thanks, > Richard. > > 2011-07-04 Richard Guenther > > PR tree-optimization/49518 > PR tree-optimization/49628 > * tree-vect-data-refs.c (vect_enhance_data_refs_alignment): Skip > irrelevant and invariant data-references. > (vect_analyze_data_ref_access): For invariant loads clear the > group association. > > * g++.dg/torture/pr49628.C: New testcase. > * gcc.dg/torture/pr49518.c: Likewise. > This caused: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50017 -- H.J.
Re: PATCH: Add -mavx2 and properly check numbers of mask bits
On Sun, 7 Aug 2011, H.J. Lu wrote: > Hi, > > opth-gen.awk has > > print "#define " mask name " (1 << " masknum[vname]++ ")" > > and int has 32bits. We should check > > if (masknum[var] > 32) > > instead of > > if (masknum[var] > 31) IIUC the (int32_t) sign-bit is supposed to be reserved and this is just a case of a missing commentarey and documentation to that effect. (Use of .opt Var being preferred.) Otherwise, you can't e.g. negate that option using the -MASK trick: see the docs. brgds, H-P
[google] Backport r174965 from trunk to google/gcc-4_6 (issue4852046)
Hi I want to backport r174965 from trunk to google/gcc-4_6, which fixed vect-72.c failure in target arm, as described in http://gcc.gnu.org/ml/gcc-patches/2011-06/msg00927.html Tested with buildit and regression test on arm qemu. OK for google/gcc-4_6 ? thanks Carrot 2011-08-08 Guozhi Wei Backport r174965 from trunk. 2011-06-12 Ira Rosen * tree-vect-data-refs.c (vect_peeling_hash_get_most_frequent): Take number of iterations to peel into account for equally frequent misalignment values. Index: tree-vect-data-refs.c === --- tree-vect-data-refs.c (revision 177320) +++ tree-vect-data-refs.c (working copy) @@ -1250,7 +1250,9 @@ vect_peeling_hash_get_most_frequent (voi vect_peel_info elem = (vect_peel_info) *slot; vect_peel_extended_info max = (vect_peel_extended_info) data; - if (elem->count > max->peel_info.count) + if (elem->count > max->peel_info.count + || (elem->count == max->peel_info.count + && max->peel_info.npeel > elem->npeel)) { max->peel_info.npeel = elem->npeel; max->peel_info.count = elem->count; -- This patch is available for review at http://codereview.appspot.com/4852046
[wwwdocs] Announce new ARM/embedded-4_6-branch branch
Hi, This new branch intends to provide fixes and enhancements for GCC 4.6 when used with ARM embedded cores. The attached patch adds documentation for this branch. Is it OK to commit? Thanks. BR, Terry svn-arm-embedded-4_6-branch.patch Description: Binary data
RE: [PATCH, ARM] Generate conditional compares in Thumb2 state
In attached new patch, arm_arch_thumb2 is changed to TARGET_THUMB2. I tried that with my patch command line option -mcpu=armv7-a9 doesn't generate IT instruction any longer, unless option "-mthumb" is being added. All of my tests assume command line option -mthumb, while cortex-M0, cortex-M3 cortex-M4 are covered by options -mcpu=cortex-m0, -march=armv7-m, and -march=armv7e-m respectively. As for the extra problem exposed by this specific case, may we treat it as a separate fix to decouple it with this one, and I can give follow up later on? I think it is a general problem not only for the particular pattern it/op/it/op. But I'm not sure how far we can go to optimize this kind of problems introduced by IT block. For this specific case, I see "if conversion" already generates conditional move before combination pass. So basically the peephole rules may probably work for most of the general scenarios. My initial thought is go over the rules introducing IT block and try to figure out all of the combination that two of this kinds of rules can be in sequential order. Maybe we only need to handle the most common case like this one. Since I do see a bunch of rules have something to do with problem, I'd like to look into all of them to give a most reasonable solution in a separate fix. Does it make sense? Thanks, -Jiangning > -Original Message- > From: Ramana Radhakrishnan [mailto:ramana.radhakrish...@linaro.org] > Sent: Friday, August 05, 2011 9:20 AM > To: Jiangning Liu > Cc: gcc-patches@gcc.gnu.org > Subject: Re: [PATCH, ARM] Generate conditional compares in Thumb2 state > > On 3 August 2011 08:48, Jiangning Liu wrote: > > This patch is to generate more conditional compare instructions in > Thumb2 > > state. Given an example like below, > > > > int f(int i, int j) > > { > > if ( (i == '+') || (j == '-') ) { > > return i; > > } else { > > return j; > > } > > } > > > > Without the patch, compiler generates the following codes, > > > > sub r2, r0, #43 > > rsbs r3, r2, #0 > > adc r3, r3, r2 > > cmp r1, #45 > > it eq > > orreq r3, r3, #1 > > cmp r3, #0 > > it eq > > moveq r0, r1 > > bx lr > > > > With the patch, compiler can generate conditional jump like below, > > > > cmp r0, #43 > > it ne > > cmpne r1, #45 > > it ne > > movne r0, r1 > > bx lr > > > Nice improvement but there could be a single it block to handle both > and thus you > could make this even better with > > cmp r0, #43 > itt ne > cmpne r1 ,#45 > movne r0, r1 > > The way to do this would be to try and split this post-reload > unfortunately into the cmp instruction and the conditional compare > with the appropriate instruction length - Then the backend has a > chance of merging some of this into a single instruction. > Unfortunately that won't be very straight-forward but that's a > direction we probably ought to proceed with in this case. > > In a number of places: > > > + if (arm_arch_thumb2) > > Ah instead of this please use if (TARGET_THUMB2) - arm_arch_thumb2 is > true based on the architecture levels and not necessarily if the user > wants to generate Thumb code. I don't want an unnecessary IT > instruction being emitted in the ASM block in ARM state for v7-a and > above. > > > Tested against arm-none-eabi target and no regression found. > > Presumably for ARM and Thumb2 state ? > > > cheers > Ramana fix_cond_cmp_2.patch Description: Binary data