Re: remove wrong code in immed_double_const
Mike Stump writes: >> We currently only support constant integer >> widths <= 2*HOST_BITS_PER_WIDE_INT, and the assert is correctly >> triggering if we try to build a wider constant. > > Can you give me a concrete example of what will fail with the constant > 0 generated by: > > return GEN_INT (i0); > > when the mode is 2*HOST_BITS_PER_WIDE_INT? When the mode is larger than this? I think the assert is more saying that things have already gone wrong if we have reached this point. immed_double_const is only designed to handle two HOST_WIDE_INTs, so what exactly is the caller trying to do? E.g. do we apply sign or zero extension to the 2 HOST_WIDE_INTs? If we're going to remove the assert, we need to define stuff like that. All ones is a pretty common constant, so if I was going to guess, I'd have expected we'd sign extend, just like we do for CONST_INT. So immed_double_const (-1, -1, int_mode) would always be GEN_INT (-1). But if we do that, we need to define the high HOST_WIDE_INT of a CONST_DOUBLE to be sign-extended too. And at the moment we don't; the CONST_DOUBLE case of simplify_immed_subreg says: /* It shouldn't matter what's done here, so fill it with zero. */ for (; i < elem_bitsize; i += value_bit) *vp++ = 0; Compare with the CONST_INT case: /* CONST_INTs are always logically sign-extended. */ for (; i < elem_bitsize; i += value_bit) *vp++ = INTVAL (el) < 0 ? -1 : 0; So... > See, we already shorten the width of wide constants and expect that to > work. This is just another instance of shortening. Now, just to see > what lurks in there, I when through 100% of the uses of CONST_DOUBLE > in *.c to get a feel for what might go wrong, the code is as benign as > I expected, every instance I saw, looked reasonably safe. It doesn't > get any better than this. ...I'm not sure about. >From a quick grep, it looks like the case where I saw the immed_double_const assert triggering -- the expansion of INTEGER_CST -- is the only case where it could trigger. So I suppose the question shifts to the tree level. Are callers to build_int_cst_wide and double_int_to_tree already expected to check that constants wider than a double_int would not be truncated? If so, then great. And if so, what are the rules there regarding sign vs. zero extension beyond the high HOST_WIDE_INT? >> FWIW, here's another case where this came up: >> >>http://gcc.gnu.org/ml/gcc-patches/2011-03/msg01220.html > > Yes, and surprisingly, or not it is the exact same case as I can into. I'd expect it to be a slightly different case. The original testcase was a union constructor that was unnecessarily initialised to zero. That has since been fixed. Richard
Re: Remove anachronistic docs about G++ template instantiation
On Thu, 8 Mar 2012, Jonathan Wakely wrote: > The manual claims a future version of G++ will support a hybrid > instantiation model, which I don't think is still planned, and > describes extern templates as an extension when they are in C++11. > > * doc/extend.texi (Template Instantiation): Remove anachronisms. I was waiting for a C++ frontend maintainer to chime in. The patch per se looks good to me, and based on some others mails around template instantiations I think you can go ahead. Gerald
Re: [C++ Patch] PR 14710 (add -Wuseless-cast)
Hi,
That won't catch something like
int i;
static_cast(i);
which is also a useless cast, because i is already an int lvalue; not
all lvalues are derived from references. Note that something like
static_cast(42);
is not a useless cast, because it turns a prvalue into an xvalue.
Agreed. In order to handle correctly such cases I modified the check per
the below. I also gated the not-so-trivial work with warn_useless_cast
and extended the testcase.
As usual, tested x86_64-linux.
Thanks,
Paolo.
//
Index: doc/invoke.texi
===
--- doc/invoke.texi (revision 185505)
+++ doc/invoke.texi (working copy)
@@ -274,8 +274,8 @@ Objective-C and Objective-C++ Dialects}.
-Wunused-label -Wunused-local-typedefs -Wunused-parameter @gol
-Wno-unused-result -Wunused-value @gol -Wunused-variable @gol
-Wunused-but-set-parameter -Wunused-but-set-variable @gol
--Wvariadic-macros -Wvector-operation-performance -Wvla
--Wvolatile-register-var -Wwrite-strings -Wzero-as-null-pointer-constant}
+-Wuseless-cast -Wvariadic-macros -Wvector-operation-performance @gol
+-Wvla -Wvolatile-register-var -Wwrite-strings -Wzero-as-null-pointer-constant}
@item C and Objective-C-only Warning Options
@gccoptlist{-Wbad-function-cast -Wmissing-declarations @gol
@@ -4199,6 +4199,11 @@ types. @option{-Wconversion-null} is enabled by de
Warn when a literal '0' is used as null pointer constant. This can
be useful to facilitate the conversion to @code{nullptr} in C++11.
+@item -Wuseless-cast @r{(C++ and Objective-C++ only)}
+@opindex Wuseless-cast
+@opindex Wno-useless-cast
+Warn when an expression is casted to its own type.
+
@item -Wempty-body
@opindex Wempty-body
@opindex Wno-empty-body
Index: c-family/c.opt
===
--- c-family/c.opt (revision 185505)
+++ c-family/c.opt (working copy)
@@ -697,6 +697,10 @@ Wzero-as-null-pointer-constant
C++ ObjC++ Var(warn_zero_as_null_pointer_constant) Warning
Warn when a literal '0' is used as null pointer
+Wuseless-cast
+C++ ObjC++ Var(warn_useless_cast) Warning
+Warn about useless casts
+
ansi
C ObjC C++ ObjC++
A synonym for -std=c89 (for C) or -std=c++98 (for C++)
Index: testsuite/g++.dg/warn/Wuseless-cast.C
===
--- testsuite/g++.dg/warn/Wuseless-cast.C (revision 0)
+++ testsuite/g++.dg/warn/Wuseless-cast.C (revision 0)
@@ -0,0 +1,123 @@
+// { dg-options "-Wuseless-cast" }
+
+template
+ void tmpl_f1(T& t)
+ {
+(int)(t);
+static_cast(t);
+reinterpret_cast(t);
+
+(int*)(&t);
+const_cast(&t);
+static_cast(&t);
+reinterpret_cast(&t);
+
+(int&)(t);
+const_cast(t);
+static_cast(t);
+reinterpret_cast(t);
+ }
+
+template
+ void tmpl_f2(T t)
+ {
+(int&)(t);
+const_cast(t);
+static_cast(t);
+reinterpret_cast(t);
+ }
+
+struct A { };
+
+template
+ void tmpl_f3(T& t)
+ {
+(A)(t);
+static_cast(t);
+
+(A*)(&t);
+const_cast(&t);
+static_cast(&t);
+reinterpret_cast(&t);
+dynamic_cast(&t);
+
+(A&)(t);
+const_cast(t);
+static_cast(t);
+reinterpret_cast(t);
+dynamic_cast(t);
+ }
+
+template
+ void tmpl_f4(T t)
+ {
+(A&)(t);
+const_cast(t);
+static_cast(t);
+reinterpret_cast(t);
+dynamic_cast(t);
+ }
+
+void f()
+{
+ int n;
+
+ (int)(n);// { dg-warning "useless cast" }
+ static_cast(n); // { dg-warning "useless cast" }
+ reinterpret_cast(n);// { dg-warning "useless cast" }
+
+ (int*)(&n); // { dg-warning "useless cast" }
+ const_cast(&n);// { dg-warning "useless cast" }
+ static_cast(&n); // { dg-warning "useless cast" }
+ reinterpret_cast(&n); // { dg-warning "useless cast" }
+
+ int& m = n;
+
+ (int&)(m); // { dg-warning "useless cast" }
+ const_cast(m); // { dg-warning "useless cast" }
+ static_cast(m);// { dg-warning "useless cast" }
+ reinterpret_cast(m); // { dg-warning "useless cast" }
+
+ tmpl_f1(m);
+
+ (int&)(n); // { dg-warning "useless cast" }
+ const_cast(n); // { dg-warning "useless cast" }
+ static_cast(n);// { dg-warning "useless cast" }
+ reinterpret_cast(n); // { dg-warning "useless cast" }
+
+ tmpl_f2(n);
+
+#ifdef __GXX_EXPERIMENTAL_CXX0X__
+ (int&&)(42);
+ static_cast(42);
+#endif
+
+ A a;
+
+ (A)(a); // { dg-warning "useless cast" }
+ static_cast(a); // { dg-warning "useless cast" }
+
+ (A*)(&a); // { dg-warning "useless cast" }
+ const_cast(&a); // { dg-warning "useless cast" }
+ static_cast(&a);// { dg-warning "useless cast" }
+ reinterpret_cast(&a); // { dg-warning "useless cast" }
+ dynamic_cast(&a); // { dg-warning "useless cast" }
+
+ A& b = a;
+
+ (A&)(b);
Re: PATCH: Properly generate X32 IE sequence
On Sat, Mar 17, 2012 at 10:50 PM, H.J. Lu wrote: > Since we must use reg64 in %fs:(%reg) memory operand like > > movq x@gottpoff(%rip),%reg64; > mov %fs:(%reg64),%reg > > this patch optimizes x32 TLS IE load and store by wrapping > %reg64 inside of UNSPEC when Pmode == SImode. OK for > trunk? Can you implement this with define_insn_and_split, like i.e. *tls_dynamic_gnu2_combine_32 ? >>> >>> I will give it a try again. Last time when I tried it, GCC didn't >>> like memory operand in DImode when Pmode == SImode. >> >> You should remove mode for tls_symbolic_operand predicate. >> > > I am testing this patch. OK for trunk if it passes all tests? No, force_reg will generate a pseudo, so this conversion is valid only for !can_create_pseudo (). At least for *tls_initial_exec_x32_store, you will need a temporary to split the pattern after reload. Uros.
Re: remove wrong code in immed_double_const
On Mar 18, 2012, at 3:16 AM, Richard Sandiford wrote: > Mike Stump writes: >>> We currently only support constant integer >>> widths <= 2*HOST_BITS_PER_WIDE_INT, and the assert is correctly >>> triggering if we try to build a wider constant. >> >> Can you give me a concrete example of what will fail with the constant >> 0 generated by: >> >> return GEN_INT (i0); >> >> when the mode is 2*HOST_BITS_PER_WIDE_INT? When the mode is larger than >> this? > > I think the assert is more saying that things have already gone wrong You apparently don't get it. Let me more forcefully state my position. No, things have not gone wrong. Let me repeat myself, again, 0 is perfectly representable in 0 bits, by induction it is perfectly representable in n+1 bits without loss. By induction, every integral size greater than 0 is also perfectly able to represent 0. If they had gone wrong, the testcase would not work, the code would not compile and I would not get valid code. > if we have reached this point. immed_double_const is only designed > to handle two HOST_WIDE_INTs, 0 fits into two HOST_WIDE_INTs. > so what exactly is the caller trying to do? Put 0 into a CONST_INT, with the result mode VOIDmode. > E.g. do we apply sign or zero extension to the 2 HOST_WIDE_INTs? Irrelevant. 0 has the same value, 0 or sign extended, honest. > If we're going to remove the assert, we need to define stuff like that. Orthogonal. The rest of the compiler defines what happens, it either is inconsistent, in which case it is by fiat, undefined, or it is consistent, in which case that consistency defines it. The compiler is free to document this in a nice way, or do, what is usually done, which is to assume everybody just knows what it does. Anyway, my point is, this routine doesn't define the data structure, and is _completely_ orthogonal to your concern. It doesn't matter if it zero extends or sign extends or is inconsistent, has bugs, doesn't have bugs, is documented, or isn't documented. In every single one of these cases, the code in the routine I am fixing, doesn't change. That is _why_ it is orthogonal. If it weren't, you'd be able to state a value for which is mattered. You can't, which is why you are wrong. If you think you are not wrong, please state a value for which it matters how it is defined. > All ones is a pretty common constant, so if I was going to guess, > I'd have expected we'd sign extend, Idle and irrelevant speculation. Let's not go down that path. > So immed_double_const (-1, -1, int_mode) would always be GEN_INT (-1). > But if we do that, we need to define the high HOST_WIDE_INT of a > CONST_DOUBLE to be sign-extended too. And at the moment we don't; Though this is orthogonal to the patch under consideration, I'd agree, defining the values as being sign extending seems reasonable to me. > the CONST_DOUBLE case of simplify_immed_subreg says: > > /* It shouldn't matter what's done here, so fill it with >zero. */ > for (; i < elem_bitsize; i += value_bit) > *vp++ = 0; And this would would have to be fixed, if _you_ wanted to define it as sign extending. Given this code, by fiat, it is defined to either be inconsistent or to be zero extending, presently. > From a quick grep, it looks like the case where I saw the immed_double_const > assert triggering -- the expansion of INTEGER_CST -- is the only case > where it could trigger. So I suppose the question shifts to the tree level. Again, irrelevant, let's not go there. I do not propose to define the data structure in my patch, nor to change the definition of the data structure. I merely propose to fix an obvious and trivial bug.
Re: [wwwdocs] Buildstat update for 4.6
On Thu, 15 Mar 2012, Tom G. Christensen wrote: > Latest results for 4.6.x > > -tgc > > Testresults for 4.6.2: > hppa2.0w-hp-hpux11.11 > i386-pc-solaris2.8 > i686-pc-linux-gnu > powerpc-apple-darwin8.11.0 > sparc-sun-solaris2.8 (2) > x86_64-apple-darwin10.8.0 > x86_64-apple-darwin11.3.0 > x86_64-unknown-linux-gnu > > Testresults for 4.6.2: > powerpc-ibm-aix5.3.0.0 (2) > x86_64-unknown-linux-gnu Thanks, Tom! This is online now. Gerald
[Patch, fortran] PR41600 - [OOP] SELECT TYPE with associate-name => exp: Arrays not supported
Dear All,
Please find attached a fix for PR41600 plus some. It is reasonably
straightforward but the following should be noted:
(i) gfc_get_vptr_from_expr exploits that seeming fact that tracing
back any class expression through TREE_OPERAND 0 eventually gets one
back to the class expression. I will root through the various
functions that operate on class objects to remove the front-endery
that does the same thing but in a much more cumbersome way;
(ii) GFC_CLASS_TYPE_P has been introduced, as it should have been in
the first place :-( Its first use is in (i);
(iii) The error that is thrown in resolve_assoc_var is necessary
because wrong code is produced at the moment since the size of the
declared type, rather than the dynamic type, is used for allocation of
the temporary. The necessary machinery is in place to fix this and I
will do so soon; and
(iv) select_type_set_tmp was broken up to make the logic more
transparent. The result is only a little longer than it would have
been without the calls to select_derived_set_tmp and
select_class_set_tmp.
There is quite a lot of retrospective tidying up to do using (i) and (ii)!
Bootstraps and regtests on FC9/x86_64 - OK for trunk?
Cheers
Paul
2012-03-18 Paul Thomas
PR fortran/41600
* trans-array.c (build_array_ref): New static function.
(gfc_conv_array_ref, gfc_get_dataptr_offset): Call it.
* trans-expr.c (gfc_get_vptr_from_expr): New function.
(gfc_conv_derived_to_class): Add a new argument for a caller
supplied vptr and use it if it is not NULL.
(gfc_conv_procedure_call): Add NULL to call to above.
symbol.c (gfc_is_associate_pointer): Return true if symbol is
a class object.
* trans-stmt.c (trans_associate_var): Handle class associate-
names.
* expr.c (gfc_get_variable_expr): Supply the array-spec if
possible.
* trans-types.c (gfc_typenode_for_spec): Set GFC_CLASS_TYPE_P
for class types.
* trans.h : Add prototypes for gfc_get_vptr_from_expr and
gfc_conv_derived_to_class. Define GFC_CLASS_TYPE_P.
* resolve.c (resolve_variable): For class arrays, ensure that
the target expression has all the necessary _data references.
(resolve_assoc_var): Throw a "not yet implemented" error for
class array selectors that need a temporary.
* match.c (copy_ts_from_selector_to_associate,
select_derived_set_tmp, select_class_set_tmp): New functions.
(select_type_set_tmp): Call one of last two new functions.
(gfc_match_select_type): Copy_ts_from_selector_to_associate is
called if associate-name is typed.
2012-03-18 Paul Thomas
PR fortran/41600
* gfortran.dg/select_type_26.f03 : New test.
Index: gcc/fortran/trans-array.c
===
*** gcc/fortran/trans-array.c (revision 185482)
--- gcc/fortran/trans-array.c (working copy)
*** add_to_offset (tree *cst_offset, tree *o
*** 3068,3073
--- 3068,3103
}
}
+
+ static tree
+ build_array_ref (tree desc, tree offset, tree decl)
+ {
+ tree tmp;
+
+ /* Class array references need special treatment because the assigned
+ type size needs to be used to point to the element. */
+ if (GFC_DESCRIPTOR_TYPE_P (TREE_TYPE (desc))
+ && TREE_CODE (desc) == COMPONENT_REF
+ && GFC_CLASS_TYPE_P (TREE_TYPE (TREE_OPERAND (desc, 0
+ {
+ tree type = gfc_get_element_type (TREE_TYPE (desc));
+ tmp = TREE_OPERAND (desc, 0);
+ tmp = gfc_get_class_array_ref (offset, tmp);
+ tmp = fold_convert (build_pointer_type (type), tmp);
+ tmp = build_fold_indirect_ref_loc (input_location, tmp);
+ }
+ else
+ {
+ tmp = gfc_conv_array_data (desc);
+ tmp = build_fold_indirect_ref_loc (input_location, tmp);
+ tmp = gfc_build_array_ref (tmp, offset, decl);
+ }
+
+ return tmp;
+ }
+
+
+
/* Build an array reference. se->expr already holds the array descriptor.
This should be either a variable, indirect variable reference or component
reference. For arrays which do not have a descriptor, se->expr will be
*** gfc_conv_array_ref (gfc_se * se, gfc_arr
*** 3195,3204
offset = fold_build2_loc (input_location, PLUS_EXPR,
gfc_array_index_type, offset, cst_offset);
! /* Access the calculated element. */
! tmp = gfc_conv_array_data (se->expr);
! tmp = build_fold_indirect_ref (tmp);
! se->expr = gfc_build_array_ref (tmp, offset, sym->backend_decl);
}
--- 3225,3231
offset = fold_build2_loc (input_location, PLUS_EXPR,
gfc_array_index_type, offset, cst_offset);
! se->expr = build_array_ref (se->expr, offset, sym->backend_decl);
}
*** gfc_get_dataptr_offset (stmtblock_t *blo
*** 6010,6019
return;
}
! tmp = gfc_conv_array_data (desc);
! tmp = build_fold_
Re: PATCH: Properly generate X32 IE sequence
On Sun, Mar 18, 2012 at 5:01 PM, Uros Bizjak wrote:
>> I am testing this patch. OK for trunk if it passes all tests?
>
> No, force_reg will generate a pseudo, so this conversion is valid only
> for !can_create_pseudo ().
>
> At least for *tls_initial_exec_x32_store, you will need a temporary to
> split the pattern after reload.
Please try attached patch. It simply throws away all recent
complications w.r.t. to thread pointer and always handles TP in
DImode.
The testcase:
--cut here--
__thread int foo __attribute__ ((tls_model ("initial-exec")));
void bar (int x)
{
foo = x;
}
int baz (void)
{
return foo;
}
--cut here--
Now compiles to:
bar:
movqfoo@gottpoff(%rip), %rax
movl%edi, %fs:(%rax)
ret
baz:
movqfoo@gottpoff(%rip), %rax
movl%fs:(%rax), %eax
ret
In effect, this always generates %fs(%rDI) and emits REX prefix before
mov/add to satisfy brain-dead linkers.
The patch is bootstrapping now on x86_64-pc-linux-gnu.
Uros.
Index: i386.md
===
--- i386.md (revision 185505)
+++ i386.md (working copy)
@@ -12836,28 +12836,6 @@
}
[(set_attr "type" "multi")])
-;; When Pmode == SImode, there may be no REX prefix for ADD. Avoid
-;; any instructions between MOV and ADD, which may interfere linker
-;; IE->LE optimization, since the last byte of the previous instruction
-;; before ADD may look like a REX prefix. This also avoids
-;; movl x@gottpoff(%rip), %reg32
-;; movl $fs:(%reg32), %reg32
-;; Since address override works only on the (reg32) part in fs:(reg32),
-;; we can't use it as memory operand.
-(define_insn "tls_initial_exec_x32"
- [(set (match_operand:SI 0 "register_operand" "=r")
- (unspec:SI
-[(match_operand 1 "tls_symbolic_operand")]
-UNSPEC_TLS_IE_X32))
- (clobber (reg:CC FLAGS_REG))]
- "TARGET_X32"
-{
- output_asm_insn
-("mov{l}\t{%%fs:0, %0|%0, DWORD PTR fs:0}", operands);
- return "add{l}\t{%a1@gottpoff(%%rip), %0|%0, %a1@gottpoff[rip]}";
-}
- [(set_attr "type" "multi")])
-
;; GNU2 TLS patterns can be split.
(define_expand "tls_dynamic_gnu2_32"
Index: i386.c
===
--- i386.c (revision 185504)
+++ i386.c (working copy)
@@ -11509,6 +11509,10 @@ ix86_decompose_address (rtx addr, struct ix86_addr
scale = 1 << scale;
break;
+ case ZERO_EXTEND:
+ op = XEXP (op, 0);
+ /* FALLTHRU */
+
case UNSPEC:
if (XINT (op, 1) == UNSPEC_TP
&& TARGET_TLS_DIRECT_SEG_REFS
@@ -12478,15 +12482,15 @@ legitimize_pic_address (rtx orig, rtx reg)
/* Load the thread pointer. If TO_REG is true, force it into a register. */
static rtx
-get_thread_pointer (bool to_reg)
+get_thread_pointer (enum machine_mode tp_mode, bool to_reg)
{
rtx tp = gen_rtx_UNSPEC (ptr_mode, gen_rtvec (1, const0_rtx), UNSPEC_TP);
- if (GET_MODE (tp) != Pmode)
-tp = convert_to_mode (Pmode, tp, 1);
+ if (GET_MODE (tp) != tp_mode)
+tp = convert_to_mode (tp_mode, tp, 1);
if (to_reg)
-tp = copy_addr_to_reg (tp);
+tp = copy_to_mode_reg (tp_mode, tp);
return tp;
}
@@ -12538,6 +12542,7 @@ legitimize_tls_address (rtx x, enum tls_model mode
{
rtx dest, base, off;
rtx pic = NULL_RTX, tp = NULL_RTX;
+ enum machine_mode tp_mode = Pmode;
int type;
switch (model)
@@ -12563,7 +12568,7 @@ legitimize_tls_address (rtx x, enum tls_model mode
else
emit_insn (gen_tls_dynamic_gnu2_32 (dest, x, pic));
- tp = get_thread_pointer (true);
+ tp = get_thread_pointer (Pmode, true);
dest = force_reg (Pmode, gen_rtx_PLUS (Pmode, tp, dest));
set_unique_reg_note (get_last_insn (), REG_EQUAL, x);
@@ -12613,7 +12618,7 @@ legitimize_tls_address (rtx x, enum tls_model mode
else
emit_insn (gen_tls_dynamic_gnu2_32 (base, tmp, pic));
- tp = get_thread_pointer (true);
+ tp = get_thread_pointer (Pmode, true);
set_unique_reg_note (get_last_insn (), REG_EQUAL,
gen_rtx_MINUS (Pmode, tmp, tp));
}
@@ -12659,27 +12664,18 @@ legitimize_tls_address (rtx x, enum tls_model mode
case TLS_MODEL_INITIAL_EXEC:
if (TARGET_64BIT)
{
+ tp_mode = DImode;
+
if (TARGET_SUN_TLS)
{
/* The Sun linker took the AMD64 TLS spec literally
and can only handle %rax as destination of the
initial executable code sequence. */
- dest = gen_reg_rtx (Pmode);
+ dest = gen_reg_rtx (tp_mode);
emit_insn (gen_tls_initial_exec_64_sun (dest, x));
return dest;
}
- else if (Pmode == SImode)
- {
- /* Always generate
- movl %fs:0, %reg32
-
Re: [patch, committed] invoke.texi: DWARF, stabs, and ELF
On Fri, 9 Mar 2012, Sandra Loosemore wrote: > Per the DWARF web site > > http://dwarfstd.org/Download.php > > the correct names of the various versions of the DWARF standard appear > to be either "DWARF Version N" or "DWARF N", rather than e.g. "DWARF2", > "DWARF-2", "dwarf2", or whatever. This patch to invoke.texi makes usages > reflect the official names. Similarly, I've fixed up a couple instances > of incorrect usage of "stabs" (official name seems to be all lowercase, > like that) and "ELF". I like the other changes, but find "DWARF Version N" with an uppercase V in "Version" a bit unnatural and would prefer to just use the lowercase "version" here. Gerald
Re: [patch, committed] invoke.texi: DWARF, stabs, and ELF
On 03/18/2012 03:00 PM, Gerald Pfeifer wrote: On Fri, 9 Mar 2012, Sandra Loosemore wrote: Per the DWARF web site http://dwarfstd.org/Download.php the correct names of the various versions of the DWARF standard appear to be either "DWARF Version N" or "DWARF N", rather than e.g. "DWARF2", "DWARF-2", "dwarf2", or whatever. This patch to invoke.texi makes usages reflect the official names. Similarly, I've fixed up a couple instances of incorrect usage of "stabs" (official name seems to be all lowercase, like that) and "ELF". I like the other changes, but find "DWARF Version N" with an uppercase V in "Version" a bit unnatural and would prefer to just use the lowercase "version" here. Personally, I agree, but I think the principle that we must prefer to call things by their proper/official names takes precedence. The DWARF documentation seems to consistently use uppercase-V "Version" in running text as well as the document titles. -Sandra
Re: [Patch, fortran] PR41600 - [OOP] SELECT TYPE with associate-name => exp: Arrays not supported
Dear Paul,
thanks for the patch.
Paul Richard Thomas wrote:
+ /* Transfer the selector typespec to the associate name. */
+
+ copy_ts_from_selector_to_associate (gfc_expr *expr1, gfc_expr *expr2)
+ {
I think it is not obvious which type spec is which. Maybe you could
add a "(expr1)" and "(expr2)" in the comment. (Alternatively, one could
rename expr1 and expr2.)
+ if (expr2->ts.type == BT_CLASS
+ && CLASS_DATA (expr2)->as
+ && expr2->ref&& expr2->ref->type == REF_ARRAY)
+ {
+ if (expr2->ref->u.ar.type == AR_FULL)
+ expr2->rank = CLASS_DATA (expr2)->as->rank;
+ else if (expr2->ref->u.ar.type == AR_SECTION)
+ expr2->rank = expr2->ref->u.ar.dimen;
+ }
I have a bad feeling about that one for code like:
dt%class(1:2)
class%class(1:2)
dt(1:2)%class
class(1:2)%class
I fear that at least one of those will fail. In any case, assuming that
- if the last ref is BT_CLASS - the expr->ref is the right one, looks
wrong. But I might have missed some fine print and it is guaranteed to
be always the correct.
+ /* Logic is a LOT clearer with separate functions for class and derived
+ type temporaries! There are not many more lines of code either. */
if (ts->type == BT_CLASS)
! tmp = select_class_set_tmp (ts);
! else
! tmp = select_derived_set_tmp (ts);
While I concur with the comment, I think one should remove it. As patch
explanation it makes sense, but as committed it is not helpful.
gfc_add_type (tmp->n.sym, ts, NULL);
! /* Copy across the array spec to the selector, taking care as to
!whether or not it is a class object or not. */
The indention looks wrong.
(iii) The error that is thrown in resolve_assoc_var is necessary
because wrong code is produced at the moment since the size of the
declared type, rather than the dynamic type, is used for allocation of
the temporary. The necessary machinery is in place to fix this and I
will do so soon
I assume that's:
! gfc_error ("CLASS selector at %L needs a temporary which is not "
!"yet implemented",&target->where);
But I think one should also look into:
! TODO Understand why class scalar expressions must be excluded. */
! if (sym->assoc&& !(sym->ts.type == BT_CLASS&& e->rank == 0))
Overall, the patch looks okay - I am just unsure about the expr2->ref
usage in copy_ts_from_selector_to_associate.
Tobias
Re: RFC: PATCH: Add -maddress-mode=short|long for x86
On Wed, 14 Mar 2012, H.J. Lu wrote: >> Apart from the above, at least invoke.texi does not define what an x32 >> environment is. Shouldn't that done somewhere (before this terminology >> is used)? > I am not sure where to put it. In any case, here is a patch to update > GCC 4.7.0 changes with link to x32 website. Please add two "the"s, once for the ABI and once for the options; fine with this change. GeraldIndex: htdocs/gcc-4.7/changes.html === RCS file: /cvs/gcc/wwwdocs/htdocs/gcc-4.7/changes.html,v retrieving revision 1.96 diff -u -p -r1.96 changes.html --- htdocs/gcc-4.7/changes.html 7 Mar 2012 14:15:35 - 1.96 +++ htdocs/gcc-4.7/changes.html 14 Mar 2012 22:31:50 - @@ -720,6 +720,8 @@ int add_values (const __flash int *p, in for Windows mingw targets. Support for new AMD family 15h processors (Piledriver core) is now available through -march=bdver2 and -mtune=bdver2 options. +Support for http://sites.google.com/site/x32abi/";>x32 psABI + is now available through -mx32 option. ...
[v3] minor simplification to std::list
Now that G++ supports it we can use a NSDMI for std::list::_M_size to
avoid needing conditional compilation to set it in the constructors.
I think the attached patch is an improvement so I plan to commit it to
trunk soon unless I hear objections.
* include/bits/stl_list.h (list::_M_size): Use NSDMI.
* testsuite/23_containers/list/requirements/dr438/assign_neg.cc:
Adjust line numbers.
* testsuite/23_containers/list/requirements/dr438/
constructor_1_neg.cc: Likewise.
* testsuite/23_containers/list/requirements/dr438/
constructor_2_neg.cc: Likewise.
* testsuite/23_containers/list/requirements/dr438/insert_neg.cc:
Likewise.
Tested x86_64.
diff --git a/libstdc++-v3/include/bits/stl_list.h
b/libstdc++-v3/include/bits/stl_list.h
index 1e760ed..634b579 100644
--- a/libstdc++-v3/include/bits/stl_list.h
+++ b/libstdc++-v3/include/bits/stl_list.h
@@ -314,26 +314,20 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
__detail::_List_node_base _M_node;
#ifdef __GXX_EXPERIMENTAL_CXX0X__
- size_t_M_size;
+ size_t_M_size = 0;
#endif
_List_impl()
: _Node_alloc_type(), _M_node()
-#ifdef __GXX_EXPERIMENTAL_CXX0X__
- , _M_size(0)
-#endif
{ }
_List_impl(const _Node_alloc_type& __a)
: _Node_alloc_type(__a), _M_node()
-#ifdef __GXX_EXPERIMENTAL_CXX0X__
- , _M_size(0)
-#endif
{ }
#ifdef __GXX_EXPERIMENTAL_CXX0X__
_List_impl(_Node_alloc_type&& __a)
- : _Node_alloc_type(std::move(__a)), _M_node(), _M_size(0)
+ : _Node_alloc_type(std::move(__a)), _M_node()
{ }
#endif
};
diff --git
a/libstdc++-v3/testsuite/23_containers/list/requirements/dr438/assign_neg.cc
b/libstdc++-v3/testsuite/23_containers/list/requirements/dr438/assign_neg.cc
index c088e6c..05664b9 100644
--- a/libstdc++-v3/testsuite/23_containers/list/requirements/dr438/assign_neg.cc
+++ b/libstdc++-v3/testsuite/23_containers/list/requirements/dr438/assign_neg.cc
@@ -18,7 +18,7 @@
//
Re: [PATCH] Straight line strength reduction, part 1
On Sun, Mar 18, 2012 at 6:12 PM, William J. Schmidt wrote: > Greetings, > > Now that we're into stage 1 again, I'd like to submit the first round of > changes for dominator-based strength reduction, which will address > issues from PR22586, PR35308, PR46556, and perhaps others. I'm > attaching two patches: the smaller (slsr-part1) is the patch I'm > submitting for approval today, while the larger (slsr-fyi) is for > reference only, but may be useful if questions arise about how the small > patch fits into the intended whole. > > This patch contains the logic for identifying strength reduction > candidates, and makes replacements only for those candidates where the > stride is a fixed constant. Replacement for candidates with fixed but > unknown strides are not implemented herein, but that logic can be viewed > in the larger patch. This patch does not address strength reduction of > data reference expressions, or candidates with conditional increments; > those issues will be dealt with in future patches. > > The cost model is built on the one used by tree-ssa-ivopts.c, and I've > added some new instruction costs to that model in place. It might > eventually be good to divorce that modeling code from IVOPTS, but that's > an orthogonal patch and somewhat messy. I think this is the wrong way to do straight line strength reduction considering we have a nice value numbering system which should be easy to extended to support it. Thanks, Andrew pinski > > Thanks, > Bill > > > gcc: > > 2012-03-18 Bill Schmidt > > * tree-pass.h (pass_strength_reduction): New decl. > * tree-ssa-loop-ivopts.c (add_cost): Remove #undef; rename to > add_regs_cost. > (multiply_regs_cost): New function. > (add_const_cost): Likewise. > (extend_or_trunc_cost): Likewise. > (negate_cost): Likewise. > (get_address_cost): Rename add_cost to add_regs_cost. > (force_expr_to_var_cost): Likewise. > (get_computation_cost_at): Likewise. > (determine_iv_cost): Likewise. > * timevar.def (TV_TREE_SLSR): New timevar. > * tree-ssa-strength-reduction.c: New. > * tree-flow.h (add_regs_cost): New decl. > (multiply_regs_cost): Likewise. > (add_const_cost): Likewise. > (extend_or_trunc_cost): Likewise. > (negate_cost): Likewise. > * Makefile.in (tree-ssa-strength-reduction.o): New dependencies. > * passes.c (init_optimization_passes): Add pass_strength_reduction. > > gcc/testsuite: > > 2012-03-18 Bill Schmidt > > * gcc.dg/tree-ssa/slsr-1.c: New test. > * gcc.dg/tree-ssa/slsr-2.c: Likewise. > * gcc.dg/tree-ssa/slsr-3.c: Likewise. > * gcc.dg/tree-ssa/slsr-4.c: Likewise. >
Re: [PATCH] Straight line strength reduction, part 1
On Sun, 2012-03-18 at 18:19 -0700, Andrew Pinski wrote: > On Sun, Mar 18, 2012 at 6:12 PM, William J. Schmidt > wrote: > > Greetings, > > > > Now that we're into stage 1 again, I'd like to submit the first round of > > changes for dominator-based strength reduction, which will address > > issues from PR22586, PR35308, PR46556, and perhaps others. I'm > > attaching two patches: the smaller (slsr-part1) is the patch I'm > > submitting for approval today, while the larger (slsr-fyi) is for > > reference only, but may be useful if questions arise about how the small > > patch fits into the intended whole. > > > > This patch contains the logic for identifying strength reduction > > candidates, and makes replacements only for those candidates where the > > stride is a fixed constant. Replacement for candidates with fixed but > > unknown strides are not implemented herein, but that logic can be viewed > > in the larger patch. This patch does not address strength reduction of > > data reference expressions, or candidates with conditional increments; > > those issues will be dealt with in future patches. > > > > The cost model is built on the one used by tree-ssa-ivopts.c, and I've > > added some new instruction costs to that model in place. It might > > eventually be good to divorce that modeling code from IVOPTS, but that's > > an orthogonal patch and somewhat messy. > > I think this is the wrong way to do straight line strength reduction > considering we have a nice value numbering system which should be easy > to extended to support it. Hi Andrew, My understanding from earlier discussions with Richard is that strength reduction within the framework of PRE/FRE has been attempted in the past, but ran aground on difficulties of globally determining the profitability of replacements. Profitability is easy to determine when the stride is constant (all replacements are either harmless or profitable), but this is not at all the case when the stride has an unknown but fixed value. (The "fyi" patch contains my logic for handling this, which is difficult to do with the slightly "myopic" nature of value numbering.) Believe me, my preference is to work within existing passes where possible, but in this case I was guided by reported past experiences of others to address this in a new pass. Thanks, Bill > > Thanks, > Andrew pinski > > > > > > Thanks, > > Bill > > > > > > gcc: > > > > 2012-03-18 Bill Schmidt > > > >* tree-pass.h (pass_strength_reduction): New decl. > >* tree-ssa-loop-ivopts.c (add_cost): Remove #undef; rename to > >add_regs_cost. > >(multiply_regs_cost): New function. > >(add_const_cost): Likewise. > >(extend_or_trunc_cost): Likewise. > >(negate_cost): Likewise. > >(get_address_cost): Rename add_cost to add_regs_cost. > >(force_expr_to_var_cost): Likewise. > >(get_computation_cost_at): Likewise. > >(determine_iv_cost): Likewise. > >* timevar.def (TV_TREE_SLSR): New timevar. > >* tree-ssa-strength-reduction.c: New. > >* tree-flow.h (add_regs_cost): New decl. > >(multiply_regs_cost): Likewise. > >(add_const_cost): Likewise. > >(extend_or_trunc_cost): Likewise. > >(negate_cost): Likewise. > >* Makefile.in (tree-ssa-strength-reduction.o): New dependencies. > >* passes.c (init_optimization_passes): Add pass_strength_reduction. > > > > gcc/testsuite: > > > > 2012-03-18 Bill Schmidt > > > >* gcc.dg/tree-ssa/slsr-1.c: New test. > >* gcc.dg/tree-ssa/slsr-2.c: Likewise. > >* gcc.dg/tree-ssa/slsr-3.c: Likewise. > >* gcc.dg/tree-ssa/slsr-4.c: Likewise. > > >
Re: [PATCH] Straight line strength reduction, part 1
I knew I was forgetting something: bootstrapped and tested with no additional regressions on powerpc64-linux-gnu... On Sun, 2012-03-18 at 20:38 -0500, William J. Schmidt wrote: > On Sun, 2012-03-18 at 18:19 -0700, Andrew Pinski wrote: > > On Sun, Mar 18, 2012 at 6:12 PM, William J. Schmidt > > wrote: > > > Greetings, > > > > > > Now that we're into stage 1 again, I'd like to submit the first round of > > > changes for dominator-based strength reduction, which will address > > > issues from PR22586, PR35308, PR46556, and perhaps others. I'm > > > attaching two patches: the smaller (slsr-part1) is the patch I'm > > > submitting for approval today, while the larger (slsr-fyi) is for > > > reference only, but may be useful if questions arise about how the small > > > patch fits into the intended whole. > > > > > > This patch contains the logic for identifying strength reduction > > > candidates, and makes replacements only for those candidates where the > > > stride is a fixed constant. Replacement for candidates with fixed but > > > unknown strides are not implemented herein, but that logic can be viewed > > > in the larger patch. This patch does not address strength reduction of > > > data reference expressions, or candidates with conditional increments; > > > those issues will be dealt with in future patches. > > > > > > The cost model is built on the one used by tree-ssa-ivopts.c, and I've > > > added some new instruction costs to that model in place. It might > > > eventually be good to divorce that modeling code from IVOPTS, but that's > > > an orthogonal patch and somewhat messy. > > > > I think this is the wrong way to do straight line strength reduction > > considering we have a nice value numbering system which should be easy > > to extended to support it. > > Hi Andrew, > > My understanding from earlier discussions with Richard is that strength > reduction within the framework of PRE/FRE has been attempted in the > past, but ran aground on difficulties of globally determining the > profitability of replacements. Profitability is easy to determine when > the stride is constant (all replacements are either harmless or > profitable), but this is not at all the case when the stride has an > unknown but fixed value. (The "fyi" patch contains my logic for > handling this, which is difficult to do with the slightly "myopic" > nature of value numbering.) > > Believe me, my preference is to work within existing passes where > possible, but in this case I was guided by reported past experiences of > others to address this in a new pass. > > Thanks, > Bill > > > > > Thanks, > > Andrew pinski > > > > > > > > > > Thanks, > > > Bill > > > > > > > > > gcc: > > > > > > 2012-03-18 Bill Schmidt > > > > > >* tree-pass.h (pass_strength_reduction): New decl. > > >* tree-ssa-loop-ivopts.c (add_cost): Remove #undef; rename to > > >add_regs_cost. > > >(multiply_regs_cost): New function. > > >(add_const_cost): Likewise. > > >(extend_or_trunc_cost): Likewise. > > >(negate_cost): Likewise. > > >(get_address_cost): Rename add_cost to add_regs_cost. > > >(force_expr_to_var_cost): Likewise. > > >(get_computation_cost_at): Likewise. > > >(determine_iv_cost): Likewise. > > >* timevar.def (TV_TREE_SLSR): New timevar. > > >* tree-ssa-strength-reduction.c: New. > > >* tree-flow.h (add_regs_cost): New decl. > > >(multiply_regs_cost): Likewise. > > >(add_const_cost): Likewise. > > >(extend_or_trunc_cost): Likewise. > > >(negate_cost): Likewise. > > >* Makefile.in (tree-ssa-strength-reduction.o): New dependencies. > > >* passes.c (init_optimization_passes): Add pass_strength_reduction. > > > > > > gcc/testsuite: > > > > > > 2012-03-18 Bill Schmidt > > > > > >* gcc.dg/tree-ssa/slsr-1.c: New test. > > >* gcc.dg/tree-ssa/slsr-2.c: Likewise. > > >* gcc.dg/tree-ssa/slsr-3.c: Likewise. > > >* gcc.dg/tree-ssa/slsr-4.c: Likewise. > > > > >
Re: [google][4.6]Make option -freorder-functions= invoke function reordering linker plugin (issue 5825054)
Updated doc/invoke.texi and submitted. Thanks, -Sri. On Sat, Mar 17, 2012 at 11:58 PM, wrote: > > Ok for google branches after updating the doc/invoke.texi file. > > David > > > http://codereview.appspot.com/5825054/
