RFA: PATCH to get_inner_reference for c++/57793
This PR isn't really a C++ issue; it affects C as well, and presumably
all other languages. A comment a few lines down says
/* Avoid returning a negative bitpos as this may wreak havoc later. */
but we were failing to avoid that in this case.
Tested x86_64-pc-linux-gnu. OK for trunk/4.8?
commit 168f0f28cf3986d0e067b640031a8baaee2d09a4
Author: Jason Merrill ja...@redhat.com
Date: Tue Jul 9 00:06:51 2013 -0400
PR c++/57793
* expr.c (get_inner_reference): Avoid returning a negative bitpos.
diff --git a/gcc/expr.c b/gcc/expr.c
index 923f59b..bbec492 100644
--- a/gcc/expr.c
+++ b/gcc/expr.c
@@ -6733,7 +6733,7 @@ get_inner_reference (tree exp, HOST_WIDE_INT *pbitsize,
tem = tem.sext (TYPE_PRECISION (sizetype));
tem = tem.lshift (BITS_PER_UNIT == 8 ? 3 : exact_log2 (BITS_PER_UNIT));
tem += bit_offset;
- if (tem.fits_shwi ())
+ if (tem.fits_shwi () !tem.is_negative())
{
*pbitpos = tem.to_shwi ();
*poffset = offset = NULL_TREE;
diff --git a/gcc/testsuite/c-c++-common/pr57793.c b/gcc/testsuite/c-c++-common/pr57793.c
new file mode 100644
index 000..7858a27
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/pr57793.c
@@ -0,0 +1,23 @@
+/* PR c++/57793 */
+
+struct A { unsigned a : 1; unsigned b : 1; };
+struct B
+{
+ unsigned char c[0x4000];
+ unsigned char d[0x4ff0];
+ struct A e;
+};
+
+void *foo (struct B *p)
+{
+ if (p-e.a)
+return (void *) 0;
+ p-e.b = 1;
+ return p-c;
+}
+
+void
+bar (struct B *p)
+{
+ foo (p);
+}
[google/4_8] Merged r200809 from gcc-4_8-branch to google/gcc-4_8EOM
-- Paul Pluzhnikov
Re: [patch] PR 57362
On 06/04/2013 02:55 PM, Sriraman Tallam wrote: Sorry for the long delay. Test case added and patch attached. OK to commit? OK. Jason
Re: Aw: Re: Re: [PATCH] FPU IEEE 754 for MIPS r5900
Jürgen Urban juergenur...@gmx.de writes: Hello, Jürgen Urban juergenur...@gmx.de writes: Jürgen Urban juergenur...@gmx.de writes: I used the SPU code in GCC as example for creating an r5900_single_format structure. The patch is attached to the e-mail. I want to submit this patch. Thanks. Are there any real differences though? E.g. in your version you set has_sign_dependent_rounding, but that's not necessary when the only rounding mode is towards zero. has_sign_dependent_rounding says whether rounding X vs. -X can give numbers of different magnitude. (It was actually because of r5900 that this distinction was added.) I'm also not sure it makes sense to choose a different NaN encoding when NaNs aren't supported anyway. It would be good if we could reuse spu_single_format directly. I don't know what the effect of has_sign_dependent_rounding is. Like I say, it tells GCC whether -X can round to something other than -Y in cases where X would round to Y. This is true for IEEE when rounding towards +infinity or -infinity, but those modes aren't supported on the R5900. Some transformations are invalid when has_sign_dependent is true. E.g. -(X - Y) is not always equal to Y - X. We want it to be false when possible, so it looked like the spu_single_format version was right. The manual says that the rounding differs in the least significant bit, so we need to assume that this can happen any time and it also leads to a different result when the sign is different. Just to be clear, do you mean different from IEEE? That doesn't matter for defining has_sign_dependent_rounding, which is comparing the R5900 rounding of intermediate result -X vs. the R5900 rounding of intermediate result X. Currently I have problems testing it, because the latest GCC which I use (svn r200583) is not able to build the Linux kernel, because it has at least 2 bugs. The first is the bug 57698 introduced between 17.06. and 26.06. It seems that no GCC developer is able to fix it. The second bug affects the dvb_demux.c from Linux 2.6.34 when -Os and -mlong is used. This triggers an sanity check in do_SUBST in file gcc/combine.c. The first bugs happens on all architectures. The second also appears with normal mipsel. There is also a bug which annoys me since years, __attribute__((aligned(16))) is not working with local variables and doesn't print a warning. This is particulary a problem when used in typedefs, because the problem is not visible. My native ps2sdk doesn't have an implementation of rounding. I don't have an environment with a combination of the correct versions of the different components for Linux. So I can't test it at the moment. OK, but the patch does need to be tested before it goes in. As far as 57698 goes, it would be fine to test with a 17.06 version. But why do you need to be able to build linux with gcc trunk to test this? The kernel doesn't use FP anyway. Can't you just use testcases running under the kernel you already have? I also can't test it, because the GCC is already not correctly working on SPU. Can you give an example? I wanted to say that I don't know what is correct or not, because the GCC is already not handling the other stuff correctly, e.g.: inf - inf = 0 nan - nan = 0 nan == nan = true (this must be false according to IEEE 754) Sorry, I was meaning in terms of source code. I still think these expressions have no meaning for R5900 floats, where there isn't an infinity or a nan to begin with. If the format doesn't have infinity to begin with, there's no right or wrong answer for inf - inf (__builtin_inff() - __builtin_inff()). 0x7f80 is not inf, so what the real FPU does for 0x7f80 doesn't affect things. When you construct 0x7f80 as a finite value it seems to be handled correctly. On spu-elf I tried: float f = 0x1.0p128f - 0x1.0p128f; and got zero as expected. Note that the equivalent on x86_64 gives a warning: warning: floating constant exceeds range of ‘float’ [-Woverflow] and produces a NaN. So it looks like this is working. According to the documentation http://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html: __builting_inf() should produce a warning if infinities are not supported. This doesn't happen, so I need to assume that GCC thinks that it is supported and emulates it, so it should work. I tried: float f = __builtin_inff (); for spu-elf and got: warning: target format does not support infinity [enabled by default] This doesn't give an error: float f = __builtin_inf (); because doubles do have infinity. Is that the problem you're seeing? It's not obvious what should happen there though -- others would know better than me. But it shouldn't occur in practice anyway. The standard INFINITY macro always uses __builtin_inff, and the code was written for that assumption: /* __builtin_inff is intended to be usable to define INFINITY on all targets. If an infinity is
[VMS] Update libiberty/makefile.vms
Add dwarfnames.obj, needed to build binutils on VMS hosts. Committed on both gcc and src trunks. Tristan. libiberty/ 2013-07-09 Tristan Gingold ging...@adacore.com * makefile.vms (OBJS): Add dwarfnames.obj Index: makefile.vms === --- makefile.vms(revision 200810) +++ makefile.vms(working copy) @@ -13,7 +13,8 @@ concat.obj,getruntime.obj,getpagesize.obj,getpwd.obj,xstrerror.obj,\ xmemdup.obj,xstrdup.obj,xatexit.obj,choose-temp.obj,fnmatch.obj,\ objalloc.obj,safe-ctype.obj,hashtab.obj,lbasename.obj,argv.obj,\ -lrealpath.obj,make-temp-file.obj,stpcpy.obj,unlink-if-ordinary.obj +lrealpath.obj,make-temp-file.obj,stpcpy.obj,unlink-if-ordinary.obj,\ +dwarfnames.obj ifeq ($(CC),gcc) CFLAGS=/include=([],[-.include])
[PING] Two patches pending
Hi, FW: [PATCH GCC]Relax the probability condition in CE pass when optimizing for code size http://gcc.gnu.org/ml/gcc-patches/2013-03/msg00969.html [PATCH ARM]Extend thumb1_reorg to save more comparison instructions http://gcc.gnu.org/ml/gcc-patches/2013-04/msg01057.html Thanks. bin
[PATCH GCC/ARM]Disable -fira-hoist-pressure on Thumb2
Hi,
For now the option -fira-hoist-pressure is enabled for all targets, but I
monitored performance regression with Os on thumb2 for some cases.
Considering that optimization has small improvement on thumb2 for code size,
here comes this patch disabling it on Thumb2 and modifying the corresponding
test cases.
Tested on arm-none-eabi/M3, is it OK?
Thanks.
bin
2013-04-25 Bin Cheng bin.ch...@arm.com
* common.opt (flag_ira_hoist_pressure): Initialize to 2.
* doc/invoke.texi (-fira-hoist-pressure): Disabled by default
on Thumb2 instruction set.
* config/arm/arm.c (arm_option_override): Disable option
-fira-hoist-pressure on Thumb2 by default.
2013-04-25 Bin Cheng bin.ch...@arm.com
* gcc.dg/hoist-register-pressure-1.c: Add -fira-hoist-pressure
option.
* gcc.dg/hoist-register-pressure-2.c: Ditto.
* gcc.dg/hoist-register-pressure-3.c: Ditto.Index: gcc/common.opt
===
--- gcc/common.opt (revision 197029)
+++ gcc/common.opt (working copy)
@@ -1397,7 +1397,7 @@ EnumValue
Enum(ira_region) String(mixed) Value(IRA_REGION_MIXED)
fira-hoist-pressure
-Common Report Var(flag_ira_hoist_pressure) Init(1) Optimization
+Common Report Var(flag_ira_hoist_pressure) Init(2) Optimization
Use IRA based register pressure calculation
in RTL hoist optimizations.
Index: gcc/config/arm/arm.c
===
--- gcc/config/arm/arm.c(revision 197029)
+++ gcc/config/arm/arm.c(working copy)
@@ -2095,6 +2095,12 @@ arm_option_override (void)
else
max_insns_skipped = current_tune-max_insns_skipped;
+ if (TARGET_THUMB2 flag_ira_hoist_pressure == 2)
+{
+ /* Don't do register pressure directed hoist on Thumb2 by default. */
+ flag_ira_hoist_pressure = 0;
+}
+
/* Hot/Cold partitioning is not currently supported, since we can't
handle literal pool placement in that case. */
if (flag_reorder_blocks_and_partition)
Index: gcc/doc/invoke.texi
===
--- gcc/doc/invoke.texi (revision 197029)
+++ gcc/doc/invoke.texi (working copy)
@@ -7160,7 +7160,7 @@ Use IRA to evaluate register pressure in the code
decisions to hoist expressions. This option usually results in smaller
code, but it can slow the compiler down.
-This option is enabled at level @option{-Os} for all targets.
+This option is enabled at level @option{-Os} for most targets.
@item -fira-loop-pressure
@opindex fira-loop-pressure
Index: gcc/testsuite/gcc.dg/hoist-register-pressure-1.c
===
--- gcc/testsuite/gcc.dg/hoist-register-pressure-1.c(revision 197029)
+++ gcc/testsuite/gcc.dg/hoist-register-pressure-1.c(working copy)
@@ -1,4 +1,4 @@
-/* { dg-options -Os -fdump-rtl-hoist } */
+/* { dg-options -Os -fira-hoist-pressure -fdump-rtl-hoist } */
/* { dg-final { scan-rtl-dump PRE/HOIST: end of bb .* copying expression
hoist { target { nonpic } } } } */
/* { dg-final { cleanup-rtl-dump hoist } } */
Index: gcc/testsuite/gcc.dg/hoist-register-pressure-2.c
===
--- gcc/testsuite/gcc.dg/hoist-register-pressure-2.c(revision 197029)
+++ gcc/testsuite/gcc.dg/hoist-register-pressure-2.c(working copy)
@@ -1,4 +1,4 @@
-/* { dg-options -Os -fdump-rtl-hoist } */
+/* { dg-options -Os -fira-hoist-pressure -fdump-rtl-hoist } */
/* { dg-final { scan-rtl-dump PRE/HOIST: end of bb .* copying expression
hoist } } */
/* { dg-final { cleanup-rtl-dump hoist } } */
Index: gcc/testsuite/gcc.dg/hoist-register-pressure-3.c
===
--- gcc/testsuite/gcc.dg/hoist-register-pressure-3.c(revision 197029)
+++ gcc/testsuite/gcc.dg/hoist-register-pressure-3.c(working copy)
@@ -1,4 +1,4 @@
-/* { dg-options -Os -fdump-rtl-hoist } */
+/* { dg-options -Os -fira-hoist-pressure -fdump-rtl-hoist } */
/* { dg-final { scan-rtl-dump PRE/HOIST: end of bb .* copying expression
hoist } } */
/* { dg-final { cleanup-rtl-dump hoist } } */
Re: [patch] regex_traits implementation
On 3 July 2013 05:15, Tim Shen wrote: All fixed. I use `_RegexMask` rather than `const _RegexMask` just because, IMO, const reference is for something very large and doesn't need to be every-byte-accessed. Add _RegexMask::_S_valid_mask = 0x3 instead of using the literal everywhere. Sorry for the delay, I missed this latest version - it looks excellent and is fine to commit. Do you have svn write access or do you need someone to commit it for you?
Re: RFC: Add of type-demotion pass
- Original Message - On Mon, 8 Jul 2013, Kai Tietz wrote: These passes are implementing type-demotion (type sinking into statements) for some gimple statements. I limitted inital implementation to the set of multiply, addition, substraction, and binary-and/or/xor. Additional this pass adds some rules to sink type-cast sequences - eg. (int) (short) x; with char as type of x. This special handing in this pass of such type-sequence simplification is necessary to avoid too complex cast-sequences by type-unsigned conversion used by this pass to avoid undefined overflow behaviour. Hello, thanks for working on this. This seems like a nice chance for me to learn, so I am trying to understand your patch, and I would be glad if you could give me a couple hints. I try. I wonder why you implemented this as a separate pass instead of adding it to tree-ssa-forwprop. demote_cast is (or should be) a special case of combine_conversions, so it would be nice to avoid the code duplication (there is also a version in fold-const.c). demote_into could be called from roughly the same place as simplify_conversion_from_bitmask. And you could reuse get_prop_source_stmt, can_propagate_from, remove_prop_source_from_use, etc. Initial patch (from last year) actual implemented that in forwprop. I was then kindly asked to put this into a separate pass. There are some good reason why forward-propagation isn't the right place for it. Eg, forwprop does type-raising by default. So by implementing type-demotion there too, would lead to raise-condition. So there would be required additionally that within forwprop a straight line-depth conversion is done for statement-lists. All this doesn't fit pretty well into current concept of forward-propagation ... The cast demotion is of course something of interest for folding and might be fitting into forward-propagation-pass too. The main cause why it is implemented within demotion pass is, that this pass introduces such cast-demotion-folding opportunities due its unsigned-type expansion. So we want to fold that within pass and not waiting until a later pass optimizes such redundant sequences away. Btw, the cast-demotion can be still improved for some cases - see here as example the gcc.target/i386/rotate-1.c testcase for such an improvement - so that some expressions of the form of (type1) (((type2) x) op ((type2) y)) can be transformed into x op y. If I understand, the main reason is because you want to go through the statements in reverse order, since this is the way the casts are being propagated (would forwprop also work, just more slowly, or would it miss opportunities across basic blocks?). It would miss some opportunities, and causes penalty on speed due concept of forwprop. I have some trouble understanding why something as complicated as build_and_add_sum (which isn't restricted to additions by the way) is needed. Could you add a comment to the code explaining why you need to insert the statements precisely there and not for instance just before their use? Is that to try and help CSE? Yes, this function is a bit more complex as actual required for now in general. Nevertheless required. And I expect that demotion-pass will improve and so even the right now unlikely cases might be required more frequent. I had in front lighter version of statement addition, but it failed in some cases.In some (rare) cases we would otherwise choose wrong basic-block to add the new cast-statements. Well, there is another place (reassoc) where you can find nearly the same function, and its needs are exactly the same as within demote-pass. I have added an additional early pass typedemote1 to this patch for simple cases types can be easily sunken into statement without special unsigned-cast for overflow-case. Jakub asked for it. Tests have shown that this pass does optimizations in pretty few cases. As example in testsuite see for example pr46867.c testcase. The second pass (I put it behind first vrp pass to avoid testcase-conflicts) uses 'unsigned'-type casts to avoid undefined overflow behavior. This pass has much more hits in standard code, I assume that, when the pass is done, we can consider removing the corresponding code from the front-ends? That would increase the hits ;-) Hmm, well possible, but unlikely. FE and ME are two pretty different things. Sadly stuff isn't as clearly separated as it should be. But well, I know there is work going on to achieve that in this area. -- Marc Glisse Kai
Re: [PATCH] Teach VRP about __builtin_{ffs,parity,popcount,clz,ctz,clrsb}{,l,ll,imax} (PR target/29776)
On 05/07/13 16:01, Jakub Jelinek wrote: Hi! Attached are two versions of a patch to teach VRP about the int bitop builtins. Both patches are identical for all builtins but __builtin_c[lt]z*, which are the only two from these that are documented to have undefined behavior on some argument (0). The first version is strict, it assumes __builtin_c[lt]z* (0) doesn't happen in valid programs, while the second one attempts to be less strict to avoid breaking stuff too much. The reason for writing the second patch is that longlong.h on various targets has stuff like: #ifdef __alpha_cix__ #define count_leading_zeros(COUNT,X)((COUNT) = __builtin_clzl (X)) #define count_trailing_zeros(COUNT,X) ((COUNT) = __builtin_ctzl (X)) #define COUNT_LEADING_ZEROS_0 64 #else and documents that if COUNT_LEADING_ZEROS_0 is defined, then count_leading_zeros (cnt, 0) should be well defined and set cnt to COUNT_LEADING_ZEROS_0. While neither gcc nor glibc use COUNT_LEADING_ZEROS_0 macro, I'm a little bit afraid some code in the wild might do, and it might even have its own copy of longlong.h, so even if we've removed those COUNT_LEADING_ZEROS_0 macros for targets that use the builtins, something could stay broken. So, what the patch does is if an optab exists for the mode of the builtin's argument and C?Z_DEFINED_VALUE_AT_ZERO is defined, then it will add that value to the range unless VR of argument is non-zero (well, it handles only a few interesting commonly used values, for CLZ only precision of the mode (seems right now when CLZ_DEFINED_VALUE_AT_ZERO is non-zero, it sets it always to bitsize of the mode, and even widening or double word narrowing expansion should maintain this property), for CTZ -1 and bitsize). If there isn't an optab for it, for CLZ it still assumes it might be bitsize, for CTZ it just assumes it is undefined behavior otherwise, because if I understand the code right, for CTZ we really could return various values for 0 without hw support for the mode, e.g. when CTZ is implemented using CLZ, it might return something, if we use wider mode hw CTZ and it would return bitsize, that would be bitsize of the wider mode etc. I bet longlong.h only uses __builtin_c?z builtins for modes actually implemented in hw anyway (otherwise it couldn't be used safely in libgcc implementation of those libcalls). Both patches have been bootstrapped/regtested on x86_64-linux and i686-linux, which one do you prefer? Jakub On ARM, CLZ has a defined result at zero (32). Furthermore, the ACLE specification defines (in the header arm_acle.h) __clz(n) as an intrinsic aimed at the CLZ instruction; __clz() has a defined result at 0. We want to use __builtin_clz as the implementation for __clz rather than inventing another one; but that would require the compiler to handle zero correctly. R.
Re: [PATCH] Teach VRP about __builtin_{ffs,parity,popcount,clz,ctz,clrsb}{,l,ll,imax} (PR target/29776)
On Tue, Jul 09, 2013 at 10:20:18AM +0100, Richard Earnshaw wrote:
On ARM, CLZ has a defined result at zero (32). Furthermore, the
ACLE specification defines (in the header arm_acle.h) __clz(n) as
an intrinsic aimed at the CLZ instruction; __clz() has a defined
result at 0. We want to use __builtin_clz as the implementation for
__clz rather than inventing another one; but that would require the
compiler to handle zero correctly.
The patch that has been committed is the conservative one, so it
handles any __builtin_clz{,l,ll,imax} (0) returning the mode bitsize
(because that is pretty much the only value used for 0 by targets if
they specify it). __builtin_ctz{,l,ll,imax} (0) is a different matter,
because the value at 0 varries a lot (-1, mode bitsize, undefined)
for the cases where there is optab, and for the case where __builtin_ctz
needs to be implemented say using clz, or in wider mode, or through library
routines you really can't expect anything meaningful.
So, for CTZ you get the target defined value for 0 if there is one only
if you have a ctz optab for that mode and CTZ_DEFINED_VALUE_AT_ZERO,
otherwise the patch treats it as undefined.
Jakub
Re: *ping* - Re: [Patch, Fortran] Add end-of-scope finalization (Part 2 of 2)
Dear Tobias,
The following patch prevents the warning to generate a FAIL:
--- /opt/gcc/_clean/gcc/testsuite/gfortran.dg/class_48.f90 2012-02-05
22:03:27.0 +0100
+++ /opt/gcc/work/gcc/testsuite/gfortran.dg/class_48.f902013-07-01
19:44:06.0 +0200
@@ -158,4 +158,6 @@ call test2 ()
call test3 ()
call test4 ()
end
+! TODO remove the kludge below after the spurious warning has been fixed.
+! { dg-prune-output .*invokes undefined behavior.* }
Cheers,
Dominique
Re: [Patch, Fortran] PR40276/PR57711 - improve generic diagnostic
Dear Tobias,
After this patch several tests related to pr20896 are now rejected,
it is probably right to issue an error for them, but I am not sure
that the message is right, e.g.
! { dg-do compile }
! Test the fix for PR20896 in which the ambiguous use
! of p was not detected.
!
! Contributed by Joost VandeVondele jv...@cam.ac.uk
!
INTERFACE g
SUBROUTINE s1(p) ! { dg-error is already being used }
INTERFACE
SUBROUTINE p
END
END INTERFACE
END
SUBROUTINE s2(p) ! { dg-error Global name }
INTERFACE
REAL FUNCTION p()
END
END INTERFACE
END
END INTERFACE
INTERFACE
REAL FUNCTION x()
END
END INTERFACE
INTERFACE
SUBROUTINE y
END
END INTERFACE
call g (x)
call g (y)
END
REAL FUNCTION x()
x = 0.1
END
SUBROUTINE y()
print *, 1.5
END
SUBROUTINE s1(p)
call p()
END
SUBROUTINE s2(p)
print *, p()
END
is rejected with
pr20896_db.f90:27.20:
SUBROUTINE y
1
Error: FUNCTION attribute conflicts with SUBROUTINE attribute in 'y' at (1)
pr20896_db.f90:31.10:
call g (y)
1
Error: Interface mismatch in dummy procedure 'p' at (1): 'y' is not a function
Note that pr20896 is still open, so this should not delay the commit, but will
require to update pr20896.
Thanks for the patch,
Dominique
Re: [patch] regex_traits implementation
On Tue, Jul 9, 2013 at 4:35 PM, Jonathan Wakely jwakely@gmail.com wrote: Sorry for the delay, I missed this latest version - it looks excellent and is fine to commit. Do you have svn write access or do you need someone to commit it for you? I don't have svn write access. Should I make a request of it, or ask someone(say Stephen, my GSoC mentor) to commit for me? -- Tim Shen
Re: [patch] regex_traits implementation
On 9 July 2013 11:13, Tim Shen wrote: On Tue, Jul 9, 2013 at 4:35 PM, Jonathan Wakely jwakely@gmail.com wrote: Sorry for the delay, I missed this latest version - it looks excellent and is fine to commit. Do you have svn write access or do you need someone to commit it for you? I don't have svn write access. Should I make a request of it, or ask someone(say Stephen, my GSoC mentor) to commit for me? It probably makes sense to get access but for now I can commit your most recent patch this evening, UK time (unless someone beats me to it.)
Re: [C++ Patch] PR 51786
On 07/09/2013 05:09 AM, Jason Merrill wrote: On 07/08/2013 07:32 PM, Paolo Carlini wrote: + (CLASS_TYPE_P (decl_specifiers.type) + || TREE_CODE (decl_specifiers.type) == ENUMERAL_TYPE)) You can use OVERLOAD_TYPE_P here if you want. Ah, I seemed to remember we had something, but stupidly grepped for CLASS_OR_ENUM_TYPE_P and the like ;) Anyway, changed and committed. Thanks! Paolo.
Re: [patch] regex_traits implementation
On 07/09/2013 06:17 AM, Jonathan Wakely wrote: On 9 July 2013 11:13, Tim Shen wrote: On Tue, Jul 9, 2013 at 4:35 PM, Jonathan Wakely jwakely@gmail.com wrote: Sorry for the delay, I missed this latest version - it looks excellent and is fine to commit. Do you have svn write access or do you need someone to commit it for you? I don't have svn write access. Should I make a request of it, or ask someone(say Stephen, my GSoC mentor) to commit for me? It probably makes sense to get access but for now I can commit your most recent patch this evening, UK time (unless someone beats me to it.) Applied (with two small trailing whitespace corrections) ;-) And a hearty *Thank You* to Tim Shen for picking this up! Ed
Re: MIPS don't gererate MUL when muliplying by constant of for 2^N +/- 1 optimizing for size
hi Richard,
I've Updated patch and added testcases basically they are the same testcase just
using different constants
I tried combining them into one testcase couldn'tget the scan-assembler stuff
working when multiple functions were in the assembler output I couldn't
couldn't work
out how to restrict the scans to only part of the assembler file. Maybe its not
possible
without some new dejagnu/expect fragments.
Graham
gcc/ChangeLog
* config/mips/mips.c (): Very slightly increase code of MUL.whren
optimizing for size.
gcc/testsuite/ChangeLog
* gcc.target/mips/mulsize-7.c: New.
* gcc.target/mips/mulsize-9.c New.
* gcc/target/mips/mulsize-15.c: New.
* gcc.target/mips/mulsize-17.c: New.
diffs
Description: Binary data
/* { dg-final { scan-assembler \t.globl\tf17 } } */
/* { dg-final { scan-assembler \tsll\t } } */
/* { dg-final { scan-assembler \taddu\t } } */
/* { dg-final { scan-assembler-not \tli\t } } */
/* { dg-final { scan-assembler-not \tmul\t } } */
int
f17(int x)
{
return x * 17;
}
/* { dg-final { scan-assembler \t.globl\tf15 } } */
/* { dg-final { scan-assembler \tsll\t } } */
/* { dg-final { scan-assembler \tsubu\t } } */
/* { dg-final { scan-assembler-not \tli\t } } */
/* { dg-final { scan-assembler-not \tmul\t } } */
int
f15(int x)
{
return x * 15;
}
/* { dg-final { scan-assembler \t.globl\tf9 } } */
/* { dg-final { scan-assembler \tsubu\t } } */
/* { dg-final { scan-assembler-not \tli\t } } */
/* { dg-final { scan-assembler-not \tmul\t } } */
int
f9(int x)
{
return x * 9;
}
/* { dg-final { scan-assembler \t.globl\tf7 } } */
/* { dg-final { scan-assembler \tsubu\t } } */
/* { dg-final { scan-assembler-not \tli\t } } */
/* { dg-final { scan-assembler-not \tmul\t } } */
int
f7(int x)
{
return x * 7;
}
Re: MIPS elimate trap-if-zero instruction if possible for divisions
Hi Richard, Jeff. Richard what's your idea for exposing things early enough so that VRP can eliminate the need for a trao-if-zero insn iif possible. Graham
Re: [PATCH] Teach VRP about __builtin_{ffs,parity,popcount,clz,ctz,clrsb}{,l,ll,imax} (PR target/29776)
On Tue, 9 Jul 2013, Richard Earnshaw wrote:
On ARM, CLZ has a defined result at zero (32). Furthermore, the ACLE
specification defines (in the header arm_acle.h) __clz(n) as an intrinsic
aimed at the CLZ instruction; __clz() has a defined result at 0. We want to
use __builtin_clz as the implementation for __clz rather than inventing
another one; but that would require the compiler to handle zero correctly.
Assuming the header will come with GCC, it can assume semantics we don't
document for user code - such as that __builtin_clz is defined at 0, if
that is the case with a given GCC version and target architecture. If the
semantics change, the header can then be changed at the same time.
(I'd still encourage user code wanting a defined value at 0 to do e.g.
static inline int clz (int n) { return n == 0 ? 32 : __builtin_clz (n); }
and hope GCC will optimize away the test for 0 when the instruction
semantics make it unnecessary - if it doesn't, it should be fixed to do
so. And it's certainly fine to put such code in an intrinsic header if
useful.)
--
Joseph S. Myers
jos...@codesourcery.com
[ping] Re: [patch 0/4] reimplement -fstrict-volatile-bitfields, v3
On 06/30/2013 09:24 PM, Sandra Loosemore wrote: Here is my third attempt at cleaning up -fstrict-volatile-bitfields. Ping? Part 1 removes the warnings and packedp flag. It is the same as in the last version, and has already been approved. I'll skip reposting it since the patch is here already: http://gcc.gnu.org/ml/gcc-patches/2013-06/msg00908.html Part 2 replaces parts 2, 3, and 4 in the last version. I've re-worked this code significantly to try to address Bernd Edlinger's comments on the last version in PR56997. Part 2: http://gcc.gnu.org/ml/gcc-patches/2013-07/msg1.html Part 3 is the test cases, which are the same as in the last version. Nobody has reviewed these but I assume they are OK if Part 2 is approved? http://gcc.gnu.org/ml/gcc-patches/2013-06/msg00912.html Part 4 is new; it makes -fstrict-volatile-bitfields not be the default for any target any more. It is independent of the other changes. Part 4: http://gcc.gnu.org/ml/gcc-patches/2013-07/msg2.html It seems that the change to the defaults in part 4 is still controversial but my understanding based on discussion of the previous version of the patches is that the maintainers were going to insist on that as a condition of getting the other bug fixes in. From my perspective, I'd be happy just to wrap up this patch series somehow or another, so please let me know if there are additional changes I need to make before this is suitable to check in. -Sandra
[patch, mips] Size optimization for MIPS
While doing some size measurements and optimization I found that using the
t0 through t7 registers on MIPS in MIPS16 mode resulted in larger code. This
is because you cannot actually do any operations on these registers in MIPS16
mode but can only move data to or from them. Compiling libraries like libjpeg
and libpng with this change saved about 1% in space. I also ran coremark, the
space saving there was smaller and the performance slowdown was around 3% so
I made the change dependent on the optimize_size flag.
I tested the change with the mips-mti-elf, running the GCC testsuite with the
-mips16 and saw no regressions. OK to checkin?
Steve Ellcey
sell...@mips.com
Steve Ellcey sell...@mips.com
* config/mips/mips.c (mips_conditional_register_usage): Do not
use t[0-7] registers in MIPS16 mode when optimizing for size.
diff --git a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c
index bd1d10b..fb89aa8 100644
--- a/gcc/config/mips/mips.c
+++ b/gcc/config/mips/mips.c
@@ -17213,6 +17213,21 @@ mips_conditional_register_usage (void)
fixed_regs[27] = call_used_regs[27] = 1;
fixed_regs[30] = call_used_regs[30] = 1;
+ /* In MIPS16 mode using the $t registers for reload results in code
+ that is larger (and slightly faster) then if we do not use them so
+if optimizing for size, do not use them. */
+ if (optimize_size)
+ {
+ fixed_regs[8] = call_used_regs[8] = 1;
+ fixed_regs[9] = call_used_regs[9] = 1;
+ fixed_regs[10] = call_used_regs[10] = 1;
+ fixed_regs[11] = call_used_regs[11] = 1;
+ fixed_regs[12] = call_used_regs[12] = 1;
+ fixed_regs[13] = call_used_regs[13] = 1;
+ fixed_regs[14] = call_used_regs[14] = 1;
+ fixed_regs[15] = call_used_regs[15] = 1;
+ }
+
/* Do not allow HI and LO to be treated as register operands.
There are no MTHI or MTLO instructions (or any real need
for them) and one-way registers cannot easily be reloaded. */
Re: [patch, mips] Size optimization for MIPS
On 07/09/2013 09:29 AM, Steve Ellcey wrote:
+ /* In MIPS16 mode using the $t registers for reload results in code
+ that is larger (and slightly faster) then if we do not use them so
+ if optimizing for size, do not use them. */
+ if (optimize_size)
+ {
+ fixed_regs[8] = call_used_regs[8] = 1;
+ fixed_regs[9] = call_used_regs[9] = 1;
+ fixed_regs[10] = call_used_regs[10] = 1;
+ fixed_regs[11] = call_used_regs[11] = 1;
+ fixed_regs[12] = call_used_regs[12] = 1;
+ fixed_regs[13] = call_used_regs[13] = 1;
+ fixed_regs[14] = call_used_regs[14] = 1;
+ fixed_regs[15] = call_used_regs[15] = 1;
+ }
Surely the registers are better used by LRA as spill than the stack.
Perhaps time better spent enabling LRA for the mips target?
r~
Re: [gomp4] Compiler side of the cancellation support
On 07/03/2013 03:39 AM, Jakub Jelinek wrote: Hi! This is the compiler side of the #pragma omp cancel and #pragma omp cancellation point support. On the library side what is needed is: 1) GOMP_cancellation_point now returns a bool (whether the relevant cancellation was observed) 2) GOMP_cancel now has two arguments instead of just one, and returns bool like GOMP_cancellation_point. If the second argument is false, it acts just like GOMP_cancellation_point, if it is true, it cancels the given construct. For both these calls the first argument is 1 for parallel cancellation, 2 for loop cancellation, 4 for sections and 8 for taskgroup cancellation. 3) GOMP_barrier_cancel which is like GOMP_barrier, but should check for pending parallel cancellation and if parallel is cancelled, should return true 4) GOMP_sections_end_cancel and GOMP_loop_end_cancel variants to the non-cancel libcalls for the cancellation checking implicit barriers The still unsolved problems are that for firstprivate/lastprivate for, copyin_ref we add an implicit barrier that isn't really in the standard and similarly for #pragma omp single copyprivate we don't use one barrier mandated by the standard, but actually two barriers. Not sure what exactly we want as the behavior for these. As some subset of threads can be canceled before reaching the unofficial barrier (say one with #pragma omp cancel parallel before reaching the omp for or omp single copyprivate) and some others with #pragma omp cancellation point parallel, while some threads hit the unofficial barrier before the cancellation (and optionally some afterwards), do we want in the library to just arrange for all barriers to be awaken and not block until the final barrier at the end of parallel is hit, and for the unofficial barriers just not to return anything, while for the official barriers (*_cancel suffixed) return true to signal jump to end of region with running dtors? Or perhaps keep track on how many threads in parallel have already observed the cancellation and wait on non-*_cancel barriers only for the rest of the threads that haven't observed it yet, and only on the *_cancel barriers observe it for all threads. Another issue is what if the dtors executed on the way contain barriers, but that is probably ruled out by the restriction that A construct that may be subject to cancellation must not encounter an orphaned cancellation point. Queuing this patch until we have a library implementation. I've committed the patch, so that I can more easily work on the library implementation. There was one minor patch conflict that needed resolving. r~
Re: [patch, mips] Size optimization for MIPS
Steve Ellcey sell...@mips.com writes:
While doing some size measurements and optimization I found that using the
t0 through t7 registers on MIPS in MIPS16 mode resulted in larger code. This
is because you cannot actually do any operations on these registers in MIPS16
mode but can only move data to or from them.
That was always the case though. These registers weren't enabled because
you can do operations on them. They were enabled because they should make
ideal spill space. Moves to and from these registers always take 2 bytes,
whereas stack spills take either 2 or 4 bytes (as well as being slower).
So it sounds like the problem is that the heuristics aren't tuned properly.
Disabling the registers seems like papering over the problem rather than
fixing it. But since no-one is likely to do the work to fix the heuristics,
that isn't a good enough reason to reject the patch.
+ /* In MIPS16 mode using the $t registers for reload results in code
+ that is larger (and slightly faster) then if we do not use them so
+ if optimizing for size, do not use them. */
+ if (optimize_size)
+ {
+ fixed_regs[8] = call_used_regs[8] = 1;
+ fixed_regs[9] = call_used_regs[9] = 1;
+ fixed_regs[10] = call_used_regs[10] = 1;
+ fixed_regs[11] = call_used_regs[11] = 1;
+ fixed_regs[12] = call_used_regs[12] = 1;
+ fixed_regs[13] = call_used_regs[13] = 1;
+ fixed_regs[14] = call_used_regs[14] = 1;
+ fixed_regs[15] = call_used_regs[15] = 1;
+ }
Please combine this with the previous block though, since the comment here
contradicts the comment there. It would also be worth saying why you
keep $24 (for CMP and CMPI) and $25 (for SVR4 PIC).
Please also run some sanity checks for -mabi=eabi and functions that
have 5+ arguments (all used) to make sure that they still work.
Thanks,
Richard
C++ PATCH for c++/57831 (ICE with using-declaration and qualified-id in template)
Here instantiating the SCOPE_REF didn't know how to handle seeing a
USING_DECL for the name. The right answer is to strip the USING_DECL
and return the identifier, as we do in other cases.
Tested x86_64-pc-linux-gnu, applying to trunk, 4.8 and 4.7.
commit 1d678e7704b74e7dfc0c927c47a0271c8d599b7c
Author: Jason Merrill ja...@redhat.com
Date: Mon Jul 8 23:36:12 2013 -0400
PR c++/57831
* pt.c (tsubst_copy): Handle USING_DECL.
diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index 7a36521..6f290f6 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -12508,6 +12508,9 @@ tsubst_copy (tree t, tree args, tsubst_flags_t complain, tree in_decl)
case TYPE_DECL:
return tsubst (t, args, complain, in_decl);
+case USING_DECL:
+ t = DECL_NAME (t);
+ /* Fall through. */
case IDENTIFIER_NODE:
if (IDENTIFIER_TYPENAME_P (t))
{
diff --git a/gcc/testsuite/g++.dg/template/using23.C b/gcc/testsuite/g++.dg/template/using23.C
new file mode 100644
index 000..abb90de
--- /dev/null
+++ b/gcc/testsuite/g++.dg/template/using23.C
@@ -0,0 +1,15 @@
+// PR c++/57831
+
+struct A {
+ void f();
+};
+template class T struct B : T {
+ typedef T base;
+ using base::f; // If I write using BT::f it's ok
+ void g( ) {
+BT::f(); // This is OK as expected
+(this-*T::f)();// This is also OK
+(this-*BT::f)(); // This causes error
+ }
+};
+template struct B A ;
Re: MIPS elimate trap-if-zero instruction if possible for divisions
Graham Stott graham.st...@btinternet.com writes: Hi Richard, Jeff. Richard what's your idea for exposing things early enough so that VRP can eliminate the need for a trao-if-zero insn iif possible. Well, I was thinking of doing it in expand. I.e. get the MIPS div*, mod* and divmod* patterns to emit an explicit trap-if-zero or branch-around- break sequence. (The div* and mod* patterns are in loongson.md.) With Jeff's comment about VRP though, it sounds like he had different ideas :-) Richard
Re: MIPS don't gererate MUL when muliplying by constant of for 2^N +/- 1 optimizing for size
Graham Stott graham.st...@btinternet.com writes: gcc/ChangeLog * config/mips/mips.c (): Very slightly increase code of MUL.whren optimizing for size. * config/mips/mips.c (mips_rtx_costs): Very slightly increase the cost of MULT when optimizing for size. gcc/testsuite/ChangeLog * gcc.target/mips/mulsize-7.c: New. * gcc.target/mips/mulsize-9.c New. * gcc/target/mips/mulsize-15.c: New. * gcc.target/mips/mulsize-17.c: New. Let's number these consecutively (1-4). Typo gcc/target/mips - gcc.target/mips. OK with those changes, thanks. Richard
C++ PATCH for c++/57551 (ICE with undefined constant expression)
In this testcase, we were assuming that if we have an INDIRECT_REF of an
ADDR_EXPR where the address operand has the same type as the ref, we
should have been able to fold that away. We were applying this
assumption to the offset case as well, for arrays. But in this testcase
we're indexing off a pointer to a non-array, so there's nothing there to
fold to. The simplest way to fix this ICE is to just drop the array
handling; I don't think it actually worked, anyway.
Tested x86_64-pc-linux-gnu, applying to trunk, 4.8, 4.7.
commit 42146ead7b57cd197746a23ce71a1feede1a1303
Author: Jason Merrill ja...@redhat.com
Date: Tue Jul 9 00:28:33 2013 -0400
PR c++/57551
* semantics.c (cxx_eval_indirect_ref): Don't try to look through
a POINTER_PLUS_EXPR for type punning diagnostic.
diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c
index 0a6c775..9e49060 100644
--- a/gcc/cp/semantics.c
+++ b/gcc/cp/semantics.c
@@ -7709,11 +7709,6 @@ cxx_eval_indirect_ref (const constexpr_call *call, tree t,
{
tree sub = op0;
STRIP_NOPS (sub);
- if (TREE_CODE (sub) == POINTER_PLUS_EXPR)
- {
- sub = TREE_OPERAND (sub, 0);
- STRIP_NOPS (sub);
- }
if (TREE_CODE (sub) == ADDR_EXPR)
{
/* We couldn't fold to a constant value. Make sure it's not
diff --git a/gcc/testsuite/g++.dg/expr/const1.C b/gcc/testsuite/g++.dg/expr/const1.C
new file mode 100644
index 000..9371b27
--- /dev/null
+++ b/gcc/testsuite/g++.dg/expr/const1.C
@@ -0,0 +1,9 @@
+// PR c++/57551
+
+extern unsigned long ADDR;
+
+unsigned long f(){
+ const unsigned long* const var=ADDR;
+ const unsigned long retval=var[1];
+ return retval;
+}
Re: MIPS elimate trap-if-zero instruction if possible for divisions
Richard, Isn't that to late for VRP its run twice before expand pass. Graham
Re: MIPS elimate trap-if-zero instruction if possible for divisions
Graham Stott graham.st...@btinternet.com writes: Isn't that to late for VRP its run twice before expand pass. Exactly why I said: With Jeff's comment about VRP though, it sounds like he had different ideas :-) I suggested expand because I'd have expected the rtl optimisers to deal with the cases your patterns were handling. Richard
Re: MIPS don't gererate MUL when muliplying by constant of for 2^N +/- 1 optimizing for size
Richard, I'll renumber then consecutively and fix the typo and change log entry before I commit Thanks Graham - Original Message - From: Richard Sandiford rdsandif...@googlemail.com To: Graham Stott graham.st...@btinternet.com Cc: gcc-patches@gcc.gnu.org gcc-patches@gcc.gnu.org Sent: Tuesday, 9 July 2013, 18:31 Subject: Re: MIPS don't gererate MUL when muliplying by constant of for 2^N +/- 1 optimizing for size Graham Stott graham.st...@btinternet.com writes: gcc/ChangeLog * config/mips/mips.c (): Very slightly increase code of MUL.whren optimizing for size. * config/mips/mips.c (mips_rtx_costs): Very slightly increase the cost of MULT when optimizing for size. gcc/testsuite/ChangeLog * gcc.target/mips/mulsize-7.c: New. * gcc.target/mips/mulsize-9.c New. * gcc/target/mips/mulsize-15.c: New. * gcc.target/mips/mulsize-17.c: New. Let's number these consecutively (1-4). Typo gcc/target/mips - gcc.target/mips. OK with those changes, thanks. Richard
C++ PATCH for c++/57545 (ICE with -g and non-type template parameter)
This is an instance where we were failing to canonicalize a template
argument: for an integer constant, we need to make sure that it has the
type of the template parameter, rather than possibly a typedef local to
another template.
Tested x86_64-pc-linux-gnu, applying to trunk, 4.8, 4.7.
commit 1f06a84c5f6716cbc23498eeb9a4dfff1203b516
Author: Jason Merrill ja...@redhat.com
Date: Tue Jul 9 02:26:39 2013 -0400
PR c++/57545
* pt.c (convert_nontype_argument) [INTEGER_CST]: Force the
argument to have the exact type of the parameter.
diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index 23229a9..877d3b7 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -5620,6 +5620,10 @@ convert_nontype_argument (tree type, tree expr, tsubst_flags_t complain)
else
return NULL_TREE;
}
+
+ /* Avoid typedef problems. */
+ if (TREE_TYPE (expr) != type)
+ expr = fold_convert (type, expr);
}
/* [temp.arg.nontype]/5, bullet 2
diff --git a/gcc/testsuite/g++.dg/debug/template2.C b/gcc/testsuite/g++.dg/debug/template2.C
new file mode 100644
index 000..9f5bcd9
--- /dev/null
+++ b/gcc/testsuite/g++.dg/debug/template2.C
@@ -0,0 +1,14 @@
+// PR c++/57545
+
+templatetypename T, long unsigned int N
+struct array {
+T data[N];
+};
+
+templatetypename T
+struct derived {
+typedef long unsigned int size_type;
+static const size_type n = 42;
+
+arrayint, n a;
+};
C++ PATCH for c++/57532 (bogus error with int() int())
In C++98 mode we were complaining about trying to apply a ref-qualifier
to the function type int() during tentative parsing; the easiest fix is
just not to try to tentatively parse ref-qualifiers in C++98 mode, which
also lets us fail the tentative parse faster.
Tested x86_64-pc-linux-gnu, applying to trunk and 4.8.
commit 7d3bb96439735e08dd02af1e6d031b9a676dacb3
Author: Jason Merrill ja...@redhat.com
Date: Tue Jul 9 02:34:46 2013 -0400
PR c++/57532
* parser.c (cp_parser_ref_qualifier_opt): Don't tentatively parse
a ref-qualifier in C++98 mode.
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index e2c3c3e..614cf43 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -17374,6 +17374,10 @@ cp_parser_ref_qualifier_opt (cp_parser* parser)
{
cp_ref_qualifier ref_qual = REF_QUAL_NONE;
+ /* Don't try to parse bitwise '' as a ref-qualifier (c++/57532). */
+ if (cxx_dialect cxx11 cp_parser_parsing_tentatively (parser))
+return ref_qual;
+
while (true)
{
cp_ref_qualifier curr_ref_qual = REF_QUAL_NONE;
diff --git a/gcc/testsuite/g++.dg/parse/ref-qual2.C b/gcc/testsuite/g++.dg/parse/ref-qual2.C
new file mode 100644
index 000..a78597b
--- /dev/null
+++ b/gcc/testsuite/g++.dg/parse/ref-qual2.C
@@ -0,0 +1,6 @@
+// PR c++/57532
+
+int main()
+{
+return (int() int());
+}
C++ PATCH for c++/57437 (wrong code with mutable lambda)
Here, we were wrongly treating a lambda capture proxy as a local
variable for the purpose of C++11 return by move. Fixed by preventing
it for proxy variables of all kinds.
Tested x86_64-pc-linux-gnu, applying to trunk, 4.8, 4.7.
commit a7dc31796b5a7aab2818fa9cb7e0f345ecc8fdf0
Author: Jason Merrill ja...@redhat.com
Date: Tue Jul 9 02:51:34 2013 -0400
PR c++/57437
* typeck.c (check_return_expr): Lambda proxies aren't eligible
for nrv or return by move.
diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c
index 462abdd..6f7d489 100644
--- a/gcc/cp/typeck.c
+++ b/gcc/cp/typeck.c
@@ -8399,7 +8399,8 @@ check_return_expr (tree retval, bool *no_warning)
VAR_P (retval)
DECL_CONTEXT (retval) == current_function_decl
! TREE_STATIC (retval)
- ! DECL_ANON_UNION_VAR_P (retval)
+ /* And not a lambda or anonymous union proxy. */
+ !DECL_HAS_VALUE_EXPR_P (retval)
(DECL_ALIGN (retval) = DECL_ALIGN (result))
/* The cv-unqualified type of the returned value must be the
same as the cv-unqualified return type of the
@@ -8444,7 +8445,7 @@ check_return_expr (tree retval, bool *no_warning)
Note that these conditions are similar to, but not as strict as,
the conditions for the named return value optimization. */
if ((cxx_dialect != cxx98)
- (VAR_P (retval)
+ ((VAR_P (retval) !DECL_HAS_VALUE_EXPR_P (retval))
|| TREE_CODE (retval) == PARM_DECL)
DECL_CONTEXT (retval) == current_function_decl
!TREE_STATIC (retval)
diff --git a/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-return1.C b/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-return1.C
new file mode 100644
index 000..4b353b6
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-return1.C
@@ -0,0 +1,26 @@
+// PR c++/57437
+// { dg-require-effective-target c++11 }
+
+struct A {
+ int i;
+
+ A(): i(42) {}
+ A(const A) = default;
+ A(A a): i(a.i) { a.i = 0; }
+};
+
+int main()
+{
+ A x;
+
+ auto y = [x] () mutable {
+x.i++;
+return x;
+ };
+
+ if (y().i != 43)
+__builtin_abort ();
+
+ if (y().i != 44)
+__builtin_abort ();
+}
C++ PATCH for c++/57526 (ICE with capture of auto* variable)
An earlier patch of mine changed lambda_capture_field_type to avoid
building a DECLTYPE_TYPE in cases where it isn't really necessary, but
it is necessary in the case of capture of an auto variable.
Tested x86_64-pc-linux-gnu, applying to trunk and 4.8.
commit 7d406a5c27c48d1d8fe3b182af72e656c8c6bcb3
Author: Jason Merrill ja...@redhat.com
Date: Tue Jul 9 02:59:52 2013 -0400
PR c++/57526
* semantics.c (lambda_capture_field_type): Build a DECLTYPE_TYPE
if the variable type uses 'auto'.
diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c
index 9e49060..b5bcb00 100644
--- a/gcc/cp/semantics.c
+++ b/gcc/cp/semantics.c
@@ -9171,7 +9171,7 @@ lambda_capture_field_type (tree expr, bool explicit_init_p)
}
else
type = non_reference (unlowered_expr_type (expr));
- if (!type || WILDCARD_TYPE_P (type))
+ if (!type || WILDCARD_TYPE_P (type) || type_uses_auto (type))
{
type = cxx_make_type (DECLTYPE_TYPE);
DECLTYPE_TYPE_EXPR (type) = expr;
diff --git a/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-auto3.C b/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-auto3.C
new file mode 100644
index 000..013ed52
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-auto3.C
@@ -0,0 +1,24 @@
+// PR c++/57526
+// { dg-require-effective-target c++11 }
+
+templateclass T
+struct A
+{
+ void bar( ) { }
+
+ void foo( )
+ {
+auto* this_ptr = this;
+auto lc = []( )
+ {
+ this_ptr-bar();
+ };
+lc();
+ }
+};
+
+int main()
+{
+ Aint a;
+ a.foo();
+}
commit 80bb7027cca5a7c0db92fb0338ae55cb44e9b3d6
Author: Jason Merrill ja...@redhat.com
Date: Tue Jul 9 02:59:52 2013 -0400
PR c++/57526
* semantics.c (lambda_capture_field_type): Build a DECLTYPE_TYPE
if the variable type uses 'auto'.
diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c
index 57700f7..3699ea5 100644
--- a/gcc/cp/semantics.c
+++ b/gcc/cp/semantics.c
@@ -9078,7 +9078,8 @@ lambda_capture_field_type (tree expr)
{
tree type;
if (type_dependent_expression_p (expr)
- !(TREE_TYPE (expr) TREE_CODE (TREE_TYPE (expr)) == POINTER_TYPE))
+ !(TREE_TYPE (expr) TREE_CODE (TREE_TYPE (expr)) == POINTER_TYPE
+ !type_uses_auto (TREE_TYPE (expr
{
type = cxx_make_type (DECLTYPE_TYPE);
DECLTYPE_TYPE_EXPR (type) = expr;
diff --git a/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-auto3.C b/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-auto3.C
new file mode 100644
index 000..013ed52
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-auto3.C
@@ -0,0 +1,24 @@
+// PR c++/57526
+// { dg-require-effective-target c++11 }
+
+templateclass T
+struct A
+{
+ void bar( ) { }
+
+ void foo( )
+ {
+auto* this_ptr = this;
+auto lc = []( )
+ {
+ this_ptr-bar();
+ };
+lc();
+ }
+};
+
+int main()
+{
+ Aint a;
+ a.foo();
+}
C++ PATCH for c++/57658 (ICE with decltype of captured variable)
I've recently adjusted finish_id_expression to return the identifier for
various uses of outer local variables that aren't captures. This is
another.
Tested x86_64-pc-linux-gnu, applying to trunk.
commit a9fe694d4756d7cd35a35c4fd8f0dd09543ba559
Author: Jason Merrill ja...@redhat.com
Date: Mon Jul 8 23:43:23 2013 -0400
PR c++/57658
* semantics.c (finish_id_expression): Return the id for an
unevaluated outer variable.
diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c
index b5bcb00..8da1af4 100644
--- a/gcc/cp/semantics.c
+++ b/gcc/cp/semantics.c
@@ -3056,15 +3056,15 @@ finish_id_expression (tree id_expression,
/* Disallow uses of local variables from containing functions, except
within lambda-expressions. */
- if (!outer_var_p (decl)
- /* It's not a use (3.2) if we're in an unevaluated context. */
- || cp_unevaluated_operand)
- /* OK. */;
- else if (TREE_STATIC (decl))
+ if (!outer_var_p (decl))
+ /* OK */;
+ else if (TREE_STATIC (decl)
+ /* It's not a use (3.2) if we're in an unevaluated context. */
+ || cp_unevaluated_operand)
{
if (processing_template_decl)
- /* For a use of an outer static var, return the identifier so
- that we'll look it up again in the instantiation. */
+ /* For a use of an outer static/unevaluated var, return the id
+ so that we'll look it up again in the instantiation. */
return id_expression;
}
else
diff --git a/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-template12.C b/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-template12.C
new file mode 100644
index 000..635af97
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-template12.C
@@ -0,0 +1,19 @@
+// PR c++/57568
+// { dg-require-effective-target c++11 }
+
+template class T
+struct remove_reference
+{ typedef int type; };
+template class T
+class X
+{
+enum Q { };
+bool f ()
+{
+Q a;
+[a]{
+typename remove_reference decltype (a) ::type t;
+};
+}
+};
+template class X int ;
Re: RFA: PATCH to restore old behavior of debug_vec_tree
On 7/8/13, Jason Merrill ja...@redhat.com wrote: Lawrence's overhaul of the debug() functions changed the behavior of the pvt macro in gdbinit.in. Previously it used print_node to dump each of the elements of the vector, but after the change it uses debug, which both prints less information by default and fails to handle many C++ tree nodes. Fixed by adding debug_raw for vectree,va_gc and changing debug_vec_tree to use that. OK for trunk? Looks good to me. -- Lawrence Crowl
[patch] fix libgcc configure for powerpc-*-eabispe
Tests for powerpc-none-eabispe fail with lots of undefined symbols for
_*_df and for __extendsfdf2, __make_fp, and __unordsf2. This patch
includes float support in libgcc for the target and eliminates those
test failures.
OK for mainline and the 4.8 branch? Tested with a cross build of
powerpc-none-eabispe from i686-pc-linux-gnu.
Janis
2013-07-09 Janis Johnson jani...@codesourcery.com
* config.host (powerpc-*-eabispe*: Add t-fdpbit to tmake_file.
Index: libgcc/config.host
===
--- libgcc/config.host (revision 200845)
+++ libgcc/config.host (working copy)
@@ -869,7 +869,7 @@
tmake_file=$tmake_file rs6000/t-netbsd rs6000/t-crtstuff
;;
powerpc-*-eabispe*)
- tmake_file=${tmake_file} rs6000/t-ppccomm rs6000/t-savresfgpr
rs6000/t-crtstuff t-crtstuff-pic
+ tmake_file=${tmake_file} rs6000/t-ppccomm rs6000/t-savresfgpr
rs6000/t-crtstuff t-crtstuff-pic t-fdpbit
extra_parts=$extra_parts crtbegin.o crtend.o crtbeginS.o crtendS.o
crtbeginT.o ecrti.o ecrtn.o ncrti.o ncrtn.o
;;
powerpc-*-eabisimaltivec*)
Re: [patch] fix libgcc configure for powerpc-*-eabispe
On Tue, Jul 9, 2013 at 4:23 PM, Janis Johnson janis_john...@mentor.com wrote: Tests for powerpc-none-eabispe fail with lots of undefined symbols for _*_df and for __extendsfdf2, __make_fp, and __unordsf2. This patch includes float support in libgcc for the target and eliminates those test failures. OK for mainline and the 4.8 branch? Tested with a cross build of powerpc-none-eabispe from i686-pc-linux-gnu. Okay. Thanks, David
Fwd: Resolving an issue of bootstrap failure from vectorization.
Repost to gcc-pa...@gcc.gnu.org. -- Forwarded message -- From: Cong Hou co...@google.com Date: Tue, Jul 9, 2013 at 1:10 PM Subject: Resolving an issue of bootstrap failure from vectorization. To: g...@gcc.gnu.org Cc: David Li davi...@google.com Hi My name is Cong Hou, and I am a Noogler working in the compiler optimization team at Google. When we were trying moving the vectorization from O3 to O2 in GCC 4.9, we met a bootstrap failure from comparison between stage 23. This failure is caused by a potential bug in GCC as stated below. In the file tree-vect-data-refs.c, there is a qsort() function call which sorts a group of data references using a comparison function called dr_group_sort_cmp(). In this function, the iterative hash values of tree nodes are used for comparisons. For a declaration tree node, its UID participates in the calculation of the hash value. However, a specific declaration may have different UIDs whether the debug information is switched on/off. In consequence, the results of comparisons may vary in stage 23 during bootstrapping. As a solution, I think we need a function to compare two tree nodes that does not rely on UIDs. An apparent idea is comparing the tree code first, and if they have the same tree code, then compare their subnodes recursively. To resolve the issue we have now, I just composed a very basic comparison function which only compares tree codes, and this patch can make the bootstrap get passed. I have attached the patch here. I am wondering if this is a valid solution and appreciate if someone could give me any feedback. Thank you! Cong patch.diff Description: Binary data
Ping Re: [PATCH, rs6000] PR target/57150, do not use VSX instructions for long double caller saves
Ping. This patch http://gcc.gnu.org/ml/gcc-patches/2013-07/msg00164.html is pending review. -- Joseph S. Myers jos...@codesourcery.com
Re: Resolving an issue of bootstrap failure from vectorization.
You found the root cause of the bootstrap profile with vectorizer turned on, but the patch does not look correct to me. The sorting needs to make sure accesses with the same (base, size, offset, step, r_w_kind) are placed together (see how the sorted array is used to build the interleaving chain). Using UIDs in the comparison is fine (as long as the relative order of UIDs do not change (when -g is flipped on/off)). David On Tue, Jul 9, 2013 at 1:55 PM, Cong Hou co...@google.com wrote: Repost to gcc-pa...@gcc.gnu.org. -- Forwarded message -- From: Cong Hou co...@google.com Date: Tue, Jul 9, 2013 at 1:10 PM Subject: Resolving an issue of bootstrap failure from vectorization. To: g...@gcc.gnu.org Cc: David Li davi...@google.com Hi My name is Cong Hou, and I am a Noogler working in the compiler optimization team at Google. When we were trying moving the vectorization from O3 to O2 in GCC 4.9, we met a bootstrap failure from comparison between stage 23. This failure is caused by a potential bug in GCC as stated below. In the file tree-vect-data-refs.c, there is a qsort() function call which sorts a group of data references using a comparison function called dr_group_sort_cmp(). In this function, the iterative hash values of tree nodes are used for comparisons. For a declaration tree node, its UID participates in the calculation of the hash value. However, a specific declaration may have different UIDs whether the debug information is switched on/off. In consequence, the results of comparisons may vary in stage 23 during bootstrapping. As a solution, I think we need a function to compare two tree nodes that does not rely on UIDs. An apparent idea is comparing the tree code first, and if they have the same tree code, then compare their subnodes recursively. To resolve the issue we have now, I just composed a very basic comparison function which only compares tree codes, and this patch can make the bootstrap get passed. I have attached the patch here. I am wondering if this is a valid solution and appreciate if someone could give me any feedback. Thank you! Cong
C++ PATCH for c++/57471 (bogus error with sizeof...)
For some reason, the parser leaves parser-scope et al filled after the
constructs that the explicit scopes apply to, and rely on later bits to
clear them. So we need to do that here.
Tested x86_64-pc-linux-gnu, applying to trunk.
commit 8b67a328cf8c69484d8971aededb22ca0de0b129
Author: Jason Merrill ja...@redhat.com
Date: Tue Jul 9 15:46:20 2013 -0400
PR c++/57471
* parser.c (cp_parser_sizeof_pack): Clear parser scopes.
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index 614cf43..4b683bf 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -23136,6 +23136,10 @@ cp_parser_sizeof_pack (cp_parser *parser)
cp_token *token = cp_lexer_peek_token (parser-lexer);
tree name = cp_parser_identifier (parser);
+ /* The name is not qualified. */
+ parser-scope = NULL_TREE;
+ parser-qualifying_scope = NULL_TREE;
+ parser-object_scope = NULL_TREE;
tree expr = cp_parser_lookup_name_simple (parser, name, token-location);
if (expr == error_mark_node)
cp_parser_name_lookup_error (parser, name, expr, NLE_NULL,
diff --git a/gcc/testsuite/g++.dg/cpp0x/variadic-sizeof2.C b/gcc/testsuite/g++.dg/cpp0x/variadic-sizeof2.C
new file mode 100644
index 000..dfc245d
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/variadic-sizeof2.C
@@ -0,0 +1,14 @@
+// PR c++/57471
+// { dg-require-effective-target c++11 }
+
+struct A
+{
+ static constexpr bool value = true;
+};
+
+templatetypename... Types
+struct B
+{
+ static_assert(A::value, );
+ static_assert(sizeof...(Types) == 0, );
+};
Re: [PATCH, rs6000] PR target/57150, do not use VSX instructions for long double caller saves
On Wed, Jul 3, 2013 at 12:09 PM, Joseph S. Myers jos...@codesourcery.com wrote: On Fri, 3 May 2013, Michael Meissner wrote: 2013-05-03 Michael Meissner meiss...@linux.vnet.ibm.com PR target/57150 * config/rs6000/rs6000.h (HARD_REGNO_CALLER_SAVE_MODE): Use DFmode to save TFmode registers and DImode to save TImode registers for caller save operations. (HARD_REGNO_CALL_PART_CLOBBERED): TFmode and TDmode do not need to mark being partially clobbered since they only use the first double word. * config/rs6000/rs6000.c (rs6000_init_hard_regno_mode_ok): TFmode and TDmode only use the upper 64-bits of each VSX register. That change has the effect that reload thinks TFmode (and no doubt TDmode) only takes two registers even when in general registers, causing a segfault in glibc's test-ldouble built for soft float. I'm testing this patch (this diff is against 4.8 branch) to fix this; at least, it fixes the glibc testing issue. Since the original patch went on 4.7 and 4.8 branches as well as trunk, I propose this patch for the branches as well as trunk. You may wish to test it in your original VSX configuration. 2013-07-03 Joseph Myers jos...@codesourcery.com * config/rs6000/rs6000.c (rs6000_init_hard_regno_mode_ok): Only adjust register size for TDmode and TFmode for VSX registers. I thought that you already were applying it, not asking for approval. The patch is okay. TFmode and TImode on VSX need some TLC. - David
Re: [patch, mips] Size optimization for MIPS
On Tue, 2013-07-09 at 18:25 +0100, Richard Sandiford wrote:
That was always the case though. These registers weren't enabled because
you can do operations on them. They were enabled because they should make
ideal spill space. Moves to and from these registers always take 2 bytes,
whereas stack spills take either 2 or 4 bytes (as well as being slower).
So it sounds like the problem is that the heuristics aren't tuned properly.
Disabling the registers seems like papering over the problem rather than
fixing it. But since no-one is likely to do the work to fix the heuristics,
that isn't a good enough reason to reject the patch.
Pre-LRA I don't think there was any good way to tell GCC to use a
register for reloads but not for anything else. I think LRA does
support this and someone here has started looking at LRA but we haven't
come up with register class definitions that make LRA work better then
the existing setup (particularly in terms of MIPS16 object size). Do
you know if anyone else has started looking at LRA on MIPS?
Please combine this with the previous block though, since the comment here
contradicts the comment there. It would also be worth saying why you
keep $24 (for CMP and CMPI) and $25 (for SVR4 PIC).
OK, I have done this (see attached new patch).
Please also run some sanity checks for -mabi=eabi and functions that
have 5+ arguments (all used) to make sure that they still work.
Thanks,
Richard
I built a MIPS GCC with newlib that could handle -mabi=eabi and ran the
GCC testsuite, it all looked good. I am pretty sure the GCC testsuite
has tests with 5+ arguments but I did a couple of hand tests as well and
they also looked good.
Steve Ellcey
sell...@mips.com
Steve Ellcey sell...@mips.com
* config/mips/mips.c (mips_conditional_register_usage): Do not
use t[0-7] registers in MIPS16 mode when optimizing for size.
diff --git a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c
index bd1d10b..4b5a62c 100644
--- a/gcc/config/mips/mips.c
+++ b/gcc/config/mips/mips.c
@@ -17199,10 +17199,16 @@ mips_conditional_register_usage (void)
}
if (TARGET_MIPS16)
{
- /* In MIPS16 mode, we permit the $t temporary registers to be used
-for reload. We prohibit the unused $s registers, since they
+ /* In MIPS16 mode, we prohibit the unused $s registers, since they
are call-saved, and saving them via a MIPS16 register would
-probably waste more time than just reloading the value. */
+ probably waste more time than just reloading the value.
+
+ We permit the $t temporary registers when optimizing for speed
+but not when optimizing for space because using them results in
+code that is larger (but faster) then not using them. We do
+allow $24 (t8) because it is used in CMP and CMPI instructions
+and $25 (t9) because it is used as the SVR4 PIC register. */
+
fixed_regs[18] = call_used_regs[18] = 1;
fixed_regs[19] = call_used_regs[19] = 1;
fixed_regs[20] = call_used_regs[20] = 1;
@@ -17212,6 +17218,17 @@ mips_conditional_register_usage (void)
fixed_regs[26] = call_used_regs[26] = 1;
fixed_regs[27] = call_used_regs[27] = 1;
fixed_regs[30] = call_used_regs[30] = 1;
+ if (optimize_size)
+ {
+ fixed_regs[8] = call_used_regs[8] = 1;
+ fixed_regs[9] = call_used_regs[9] = 1;
+ fixed_regs[10] = call_used_regs[10] = 1;
+ fixed_regs[11] = call_used_regs[11] = 1;
+ fixed_regs[12] = call_used_regs[12] = 1;
+ fixed_regs[13] = call_used_regs[13] = 1;
+ fixed_regs[14] = call_used_regs[14] = 1;
+ fixed_regs[15] = call_used_regs[15] = 1;
+ }
/* Do not allow HI and LO to be treated as register operands.
There are no MTHI or MTLO instructions (or any real need
Re: MIPS elimate trap-if-zero instruction if possible for divisions
On 07/09/2013 11:28 AM, Richard Sandiford wrote: Graham Stott graham.st...@btinternet.com writes: Hi Richard, Jeff. Richard what's your idea for exposing things early enough so that VRP can eliminate the need for a trao-if-zero insn iif possible. Well, I was thinking of doing it in expand. I.e. get the MIPS div*, mod* and divmod* patterns to emit an explicit trap-if-zero or branch-around- break sequence. (The div* and mod* patterns are in loongson.md.) With Jeff's comment about VRP though, it sounds like he had different ideas :-) Thinking through it a bit more, expand is probably better -- assuming the bits to attach VRP info to SSA_NAMEs and work out as we hope. jeff
Re: [PATCH] Fix the bug to check if lookup_stmt_eh_lp returns positive instead of non-zero
I tried to make a small testcase to reproduce this. But the GCC EH
handling of ERT_MUST_NOT_THROW is not working properly. In my
unittest, it will not set this flag to the destructor... Thus the
problem cannot be reproduced.
Dehao
On Mon, Jul 8, 2013 at 9:33 PM, Xinliang David Li davi...@google.com wrote:
Is it possible to add a test case?
David
On Mon, Jul 8, 2013 at 5:55 PM, Dehao Chen de...@google.com wrote:
In lookup_stmt_eh_lp, negative return value indicates a MUST_NOT_THROW
region index. In this case, we should *not* add an EH edge during VPT.
Bootstrapped and passed regression test.
OK for trunk?
Thanks,
Dehao
gcc/ChangeLog:
2013-07-08 Dehao Chen (de...@google.com)
* value-prof.c (gimple_ic): Fix the bug of adding EH edge.
Index: gcc/value-prof.c
===
--- gcc/value-prof.c (revision 200375)
+++ gcc/value-prof.c (working copy)
@@ -1359,8 +1359,7 @@ gimple_ic (gimple icall_stmt, struct cgraph_node *
/* Build an EH edge for the direct call if necessary. */
lp_nr = lookup_stmt_eh_lp (icall_stmt);
- if (lp_nr != 0
- stmt_could_throw_p (dcall_stmt))
+ if (lp_nr 0 stmt_could_throw_p (dcall_stmt))
{
edge e_eh, e;
edge_iterator ei;
Re: RFC: Add of type-demotion pass
On 07/08/2013 02:52 PM, Marc Glisse wrote: I wonder why you implemented this as a separate pass instead of adding it to tree-ssa-forwprop. demote_cast is (or should be) a special case of combine_conversions, so it would be nice to avoid the code duplication (there is also a version in fold-const.c). demote_into could be called from roughly the same place as simplify_conversion_from_bitmask. And you could reuse get_prop_source_stmt, can_propagate_from, remove_prop_source_from_use, etc. That's a real good question; I find myself looking a lot at the bits in forwprop and I'm getting worried it's on its way to being an unmaintainable mess. Sadly, I'm making things worse rather than better with my recent changes. I'm still hoping more structure will become evident as I continue to work through various improvements. I find myself also pondering these bits in a code motion model; what hasn't become clear yet is the driving motivation to show why thinking about this as a code motion problem is interesting. Conceptually we can hoist casts to their earliest possible point and sink them to their latest possible point. What are the benefits of those transformations and is there anything inherently good about actually moving the typecasts as opposed to just realizing the casts are in the IL and optimizing appropriately. ie, often I see the hoisting/sinking code bring a series of casts together into straighline code which then gets optimized. *BUT* is there anything inherently better/easier with having them in straightline code. We can walk the use-def chains and recover the same information. If that's not happening, then that points to a failing in our optimizers. Floating out there is the hope that there's a set of canonicalization rules to guide us where to place the typecasts. ie, is it generally better to have (T) (a) OP (T) b Or is it better to have (T) (a OP b) [ Assuming they're semantically the same. ] Is it dependent on T and how T relates to the underlying target? Are the guidelines likely the same for logicals vs arithmetic, etc? If I understand, the main reason is because you want to go through the statements in reverse order, since this is the way the casts are being propagated (would forwprop also work, just more slowly, or would it miss opportunities across basic blocks?). SSA_NAME_DEF_STMT can cross block boundaries. I have some trouble understanding why something as complicated as build_and_add_sum (which isn't restricted to additions by the way) is needed. Could you add a comment to the code explaining why you need to insert the statements precisely there and not for instance just before their use? Is that to try and help CSE? Hmm, I thought I had suggested that routine get renamed during an internal, informal review of Kai's work. I have added an additional early pass typedemote1 to this patch for simple cases types can be easily sunken into statement without special unsigned-cast for overflow-case. Jakub asked for it. Tests have shown that this pass does optimizations in pretty few cases. As example in testsuite see for example pr46867.c testcase. The second pass (I put it behind first vrp pass to avoid testcase-conflicts) uses 'unsigned'-type casts to avoid undefined overflow behavior. This pass has much more hits in standard code, I assume that, when the pass is done, we can consider removing the corresponding code from the front-ends? That would increase the hits ;-) Kai and I have briefly touched on this, mostly in the context of removing bits from fold-const.c rather than the front-ends proper. jeff
Re: [PATCH 1/6] Andes nds32: configure settings for nds32 target.
On Mon, 8 Jul 2013, Chung-Ju Wu wrote:
+ nds32*-*-*)
+ supported_defaults=arch nds32_lib
+
+ # process --with-arch
+ # the 'with_arch' will be analyzed and then set its ISA and
FLAGS
I don't think this approach for processing --with-arch and setting
target_cpu_default in a complicated way based on it is a good idea.
The normal handling of --with-arch is to generate a -march= option via
OPTION_DEFAULT_SPECS. If you do that, then the conversion from -march=
strings to sets of options enabled internally is done within the compiler
proper.
+nds32*-elf*)
+ # Basic makefile fragment and extra_parts for crt stuff.
+ tmake_file=${tmake_file} nds32/t-nds32
+ extra_parts=crtbegin1.o crtend1.o
+ # Append extra c-isr library according to --with-arch=X setting.
Now, given that you add a -march= option, there's the possibility that
someone might do a build with multilibs for different -march= values. So
the choice here (libgcc configuration) needs to depend on the actual
configuration of the compiler for the multilib being built, probably via
configure tests in libgcc/configure.ac.
--
Joseph S. Myers
jos...@codesourcery.com
Restore cr16-elf
Hi! I just tried to build cr16-elf, which fails due to a dependency problem: `gencodes' is needed to build insn-codes.h, but for building it, target.h is included in cr16-protos.h, which in turn includes insn-codes.h: [...] g++ -c -g -O2 -DIN_GCC -DCROSS_DIRECTORY_STRUCTURE -fno-exceptions -fno-rtti -fasynchronous-unwind-tables -W -Wall -Wno-narrowing -Wwrite-strings -Wcast-qual -Wmissing-format-attribute -pedantic -Wno-long-long -Wno-variadic-macros -Wno-overlength-strings -fno-common -DHAVE_CONFIG_H -DGENERATOR_FILE -I. -Ibuild -I../../../../gcc/gcc -I../../../../gcc/gcc/build -I../../../../gcc/gcc/../include -I../../../../gcc/gcc/../libcpp/include -I../../../../gcc/gcc/../libdecnumber -I../../../../gcc/gcc/../libdecnumber/dpd -I../libdecnumber -I../../../../gcc/gcc/../libbacktrace\ -o build/gencondmd.o build/gencondmd.c In file included from ../../../../gcc/gcc/config/cr16/cr16-protos.h:24:0, from ./tm_p.h:4, from build/gencondmd.c:24: ../../../../gcc/gcc/target.h:52:24: fatal error: insn-codes.h: No such file or directory compilation terminated. make[2]: *** [build/gencondmd.o] Error 1 make[2]: Leaving directory `/mnt/devel/src/linux/build/cr16-elf/gcc-stage1/gcc' make[1]: *** [all-gcc] Error 2 make[1]: Leaving directory `/mnt/devel/src/linux/build/cr16-elf/gcc-stage1' The target.h include in cr16-protos.h isn't needed, so don't include it. This allows to build for cr16-elf again. Ok to commit? 2013-07-10 Jan-Benedict Glaw jbg...@lug-owl.de gcc/ * config/cr16/cr16-protos.h: Don't include target.h. diff --git a/gcc/config/cr16/cr16-protos.h b/gcc/config/cr16/cr16-protos.h index a521315..f6663c9 100644 --- a/gcc/config/cr16/cr16-protos.h +++ b/gcc/config/cr16/cr16-protos.h @@ -21,8 +21,6 @@ #ifndef GCC_CR16_PROTOS_H #define GCC_CR16_PROTOS_H -#include target.h - /* Register usage. */ extern enum reg_class cr16_regno_reg_class (int); extern int cr16_hard_regno_mode_ok (int regno, enum machine_mode); -- Jan-Benedict Glaw jbg...@lug-owl.de +49-172-7608481 Signature of: GDB has a 'break' feature; why doesn't it have 'fix' too? the second : signature.asc Description: Digital signature
Re: [PATCH 2/6] Andes nds32: machine description of nds32 porting (1).
On Mon, 8 Jul 2013, Chung-Ju Wu wrote:
+/*
*/
+
+/*++*
+ *||*
+ *| This file is divided into six parts: |*
+ *||*
+ *| PART 1: Auxiliary static function and variable declarations. |*
+ *||*
+ *| PART 2: Target hooks static function and variable declarations. |*
+ *||*
+ *| PART 3: Initialize target hooks structure and definitions. |*
+ *||*
+ *| PART 4: Implement auxiliary static function definitions, |*
+ *| the prototype is in nds32.c. |*
+ *||*
+ *| PART 5: Implement target hook stuff definitions, |*
+ *| the prototype is in nds32.c. |*
+ *||*
+ *| PART 6: Implement extern function definitions, |*
+ *| the prototype is in nds32-protos.h. |*
+ *||*
+
*++*/
Fancy comment formatting like this is not the norm in GCC. Comments
should just look like
/* Comment text.
Subsequent lines lined up like this.
End of comment. */
I also advise topologically sorting static functions and variables and
putting the target hook structure initialization at the end of the file,
to minimize the number of static declarations needed separate from the
definitions (they should generally be needed only for functions called
recursively).
+/* Define intrinsic register names.
+ Please refer to nds32_intrinsic.h file, the index is corresponding to
+ 'enum nds32_intrinsic_registers' data type values.
+ NOTE that the base value starting from 1024. */
+static const char* nds32_intrinsic_register_names[] =
I suspect the array here can itself be made const.
+/* 17.8 Register Classes */
Please avoid all comments referring to section numbers in particular
versions of the manual; there's no way those comments are ever going to
get updated when sections are added, removed, renamed or reordered.
+ /* Prepare jmptbl section and symbol name. */
+ sprintf (section_name, .nds32_jmptbl.%02d, vector_id);
+ sprintf (symbol_name, _nds32_jmptbl_%02d, vector_id);
Anywhere you use sprintf, please use snprintf instead (with the size
argument generally being sizeof (buffer_variable), as good coding practice
to avoid buffer overruns if there was a mistake in the reasoning that the
buffer size was big enough.
+ /* Currently, we don't support PIC code generation yet. */
+ if (flag_pic)
+error (not support -fpic);
Something not supported yet sounds like a case for sorry () instead of
error ().
+/* For the following option pairs:
+ 1. (-mbig-endian, -mlittle-endian)
+ 2. (-mreduced-regs, -mfull-regs)
+ We reject them to appear in the command line. */
+#define CC1_SPEC
\
+ %{mbig-endian:
\
+ %{mlittle-endian:%emay not use both -mbig-endian and
-mlittle-endian}} \
+ %{mreduced-regs:
\
+ %{mfull-regs:%emay not use both -mreduced-regs and -mfull-regs}}
If you put these options in Negative() loops in the .opt file, then you
can have the last option on the command line winning (with only the last
option being visible to specs), which is the usual practice, instead of
having such errors.
+#define LINK_SPEC \
+ %{v} %{mbig-endian:-EB} %{mlittle-endian:-EL}
Do you really need %{v} here? Such things shouldn't generally be
architecture-specific.
--
Joseph S. Myers
jos...@codesourcery.com
Re: [PATCH 2/6] Andes nds32: machine description of nds32 porting (2).
On Mon, 8 Jul 2013, Chung-Ju Wu wrote: +/* This is used to identify used ISA when doing code generation. + Initialize it with macro TARGET_DEFAULT_ISA, + which is defined in nds32-isa-xxx.h file. + User can specify it by using '-misa=X' option. */ +enum nds32_isa nds32_selected_isa = TARGET_DEFAULT_ISA; Rather than using global state, put this in the gcc_options structure using a Variable entry in your .opt file. + warning (0, For the option -misr-vector-size=X, the valid X + must be: 4 or 16); The diagnostics in this function should all not start with a capital letter. Invalid arguments to an option should be errors, not warnings. Since you have a location passed to this function, use error_at rather than the legacy functions that implicitly use input_location. +case OPT_misa_: + /* Check valid ISA: v2 v3 v3m. */ + if (strcmp (arg, v2) == 0) Use Enum in the .opt file and get all the conversion from strings to integer values, and errors for unknown values, done automatically. -- Joseph S. Myers jos...@codesourcery.com
Re: Restore cr16-elf
On Wed, 2013-07-10 01:05:21 +0200, Jan-Benedict Glaw jbg...@lug-owl.de wrote: [...] The email address listed in MAINTAINERS for the person who submitted this port, Jayant Sonar jayant.so...@kpitcummins.com, bounces, though. MfG, JBG -- Jan-Benedict Glaw jbg...@lug-owl.de +49-172-7608481 Signature of: ...und wenn Du denkst, es geht nicht mehr, the second : kommt irgendwo ein Lichtlein her. signature.asc Description: Digital signature
[C++ Patch] PR 57869
Hi,
Daniel noticed that, per to the resolution of DR195, we shouldn't warn
in c++11 mode, not even with -pedantic, for this kind of
reinterpret_cast (*). Straightforward change tested x86_64-linux, not
sure if we want to do something slightly different for C++98, or
something more.
Thanks,
Paolo.
(*) Current clang++ concurs.
///
/cp
2013-07-10 Paolo Carlini paolo.carl...@oracle.com
PR c++/57869
* typeck.c (build_reinterpret_cast_1): In c++11 mode even with
-pedantic don't warn about casting between pointer-to-function
and pointer-to-object.
/testsuite
2013-07-10 Paolo Carlini paolo.carl...@oracle.com
PR c++/57869
* g++.dg/cpp0x/reinterpret_cast1.C: New.
* g++.dg/conversion/dr195.C: Update.
* g++.dg/expr/cast2.C: Likewise.
Index: cp/typeck.c
===
--- cp/typeck.c (revision 200852)
+++ cp/typeck.c (working copy)
@@ -6722,12 +6722,15 @@ build_reinterpret_cast_1 (tree type, tree expr, bo
else if ((TYPE_PTRFN_P (type) TYPE_PTROBV_P (intype))
|| (TYPE_PTRFN_P (intype) TYPE_PTROBV_P (type)))
{
- if (pedantic (complain tf_warning))
+ if (cxx_dialect == cxx98
+ pedantic (complain tf_warning))
/* Only issue a warning, as we have always supported this
where possible, and it is necessary in some cases. DR 195
- addresses this issue, but as of 2004/10/26 is still in
- drafting. */
- warning (0, ISO C++ forbids casting between pointer-to-function and
pointer-to-object);
+ addressed this issue, and C++11 5.2.10 p8 now says that
+ Converting a function pointer to an object pointer type
+ or vice versa is conditionally-supported. */
+ warning (0, ISO C++ 1998 forbids casting between pointer-to-function
+and pointer-to-object);
return fold_if_not_in_template (build_nop (type, expr));
}
else if (TREE_CODE (type) == VECTOR_TYPE)
Index: testsuite/g++.dg/conversion/dr195.C
===
--- testsuite/g++.dg/conversion/dr195.C (revision 200852)
+++ testsuite/g++.dg/conversion/dr195.C (working copy)
@@ -2,10 +2,11 @@
// Contributed by Nathan Sidwell 20 Oct 2004 nat...@codesourcery.com
// DR 195 will allow conversions between function and object pointers
-// under some circumstances. It is in drafting, so we don't implement
-// it (yet).
+// under some circumstances. The issue got resolved for C++11, which,
+// in 5.2.10 p8 says that: Converting a function pointer to an object
+// pointer type or vice versa is conditionally-supported. */
-// This checks we warn when being pedantic.
+// This checks we warn when being pedantic in c++98 mode.
typedef void (*PF)(void);
typedef void *PV;
@@ -18,12 +19,12 @@ void foo ()
PO po;
/* the following two will almost definitly be ok with 195. */
- pf = reinterpret_cast PF(pv); // { dg-warning casting between }
- pv = reinterpret_cast PV(pf); // { dg-warning casting between }
+ pf = reinterpret_cast PF(pv); // { dg-warning casting between {
target c++98 } }
+ pv = reinterpret_cast PV(pf); // { dg-warning casting between {
target c++98 } }
/* the following two might or might not be ok with 195. */
- pf = reinterpret_cast PF(po); // { dg-warning casting between }
- po = reinterpret_cast PO(pf); // { dg-warning casting between }
+ pf = reinterpret_cast PF(po); // { dg-warning casting between {
target c++98 } }
+ po = reinterpret_cast PO(pf); // { dg-warning casting between {
target c++98 } }
/* These will never be ok, as they are implicit. */
pv = pf; // { dg-error invalid conversion }
Index: testsuite/g++.dg/cpp0x/reinterpret_cast1.C
===
--- testsuite/g++.dg/cpp0x/reinterpret_cast1.C (revision 0)
+++ testsuite/g++.dg/cpp0x/reinterpret_cast1.C (working copy)
@@ -0,0 +1,6 @@
+// PR c++/57869
+// { dg-do compile { target c++11 } }
+
+void* po = 0;
+void (*pf)() = reinterpret_castdecltype(pf)(po);
+static_assert(sizeof(po) = sizeof(pf), Conversion not supported);
Index: testsuite/g++.dg/expr/cast2.C
===
--- testsuite/g++.dg/expr/cast2.C (revision 200852)
+++ testsuite/g++.dg/expr/cast2.C (working copy)
@@ -1,5 +1,5 @@
void (*p)();
void f() {
- (void *)p; // { dg-warning forbids cast }
+ (void *)p; // { dg-warning forbids cast { target c++98 } }
}
Re: [PATCH 2/6] Andes nds32: machine description of nds32 porting (3).
The nds32_intrinsic.h header, being installed for users to use, should have the libgcc license exception. You shouldn't need the separate t-mlibs-* files for each default endianness. Just list both options in MULTILIB_OPTIONS, then define MULTILIB_DEFAULTS appropriately depending on the default endianness. -- Joseph S. Myers jos...@codesourcery.com
Re: [PATCH 3/6] Andes nds32: libgcc of nds32 porting.
There are lots of files in this patch with no copyright or license notices. GNU policy is that every file of more than ten lines needs copyright and license notices (with the libgcc license exception in this case), or a notice that the file is in the public domain. License notices should be GPLv3+exception, not GPLv2 with the old version of the exception which you have in libgcc/config/nds32/lib1asmsrc-mculib.S and some other files. -- Joseph S. Myers jos...@codesourcery.com
Re: [PATCH 4/6] Andes nds32: testsuite modifications for nds32 target.
On Mon, 8 Jul 2013, Chung-Ju Wu wrote: * gcc.dg/ucnid-11.c: Skip for nds32*-*-*. * gcc.dg/ucnid-2.c: Skip for nds32*-*-*. * gcc.dg/ucnid-3.c: Skip for nds32*-*-*. Skipping these tests seems dubious - you should fix the assembler bug that it fails to support extended identifiers instead of skipping the tests. I don't see any sign of new architecture-specific testcases. It's generally a good idea to have such tests, to verify that built-in functions and attributes and options work as expected, that the diagnostics in the back end are given for the invalid input you expect to give such diagnostics, etc. (Of course the gcc.target/nds32 testsuite will grow over time with testcases for bugs in such architecture-specific functionality that get found and fixed over time.) -- Joseph S. Myers jos...@codesourcery.com
Re: [PATCH 5/6] Andes nds32: documentation for nds32 target.
Don't you have built-in functions to document as well? Use @: (or a comma) after i.e. in Texinfo sources. The range 16-47 should use an en dash (so 16--47). -- Joseph S. Myers jos...@codesourcery.com
C++ PATCH for c++/57402 (ICE with VLA initialization)
build_vec_init wasn't handling VLAs very well.
Tested x86_64-pc-linux-gnu, applying to trunk.
commit 8c0d85d60fa6e49ce4c8ac0eae88b77b4edd98fa
Author: Jason Merrill ja...@redhat.com
Date: Tue Jul 9 18:38:13 2013 -0400
PR c++/57402
* init.c (build_vec_init): Don't take shortcuts when initializing
a VLA.
diff --git a/gcc/cp/init.c b/gcc/cp/init.c
index 58cec9e..2096230 100644
--- a/gcc/cp/init.c
+++ b/gcc/cp/init.c
@@ -,6 +,7 @@ build_vec_init (tree base, tree maxindex, tree init,
if (init
TREE_CODE (atype) == ARRAY_TYPE
+ TREE_CONSTANT (maxindex)
(from_array == 2
? (!CLASS_TYPE_P (inner_elt_type)
|| !TYPE_HAS_COMPLEX_COPY_ASSIGN (inner_elt_type))
@@ -3453,6 +3454,7 @@ build_vec_init (tree base, tree maxindex, tree init,
tree field, elt;
/* Should we try to create a constant initializer? */
bool try_const = (TREE_CODE (atype) == ARRAY_TYPE
+ TREE_CONSTANT (maxindex)
(literal_type_p (inner_elt_type)
|| TYPE_HAS_CONSTEXPR_CTOR (inner_elt_type)));
/* If the constructor already has the array type, it's been through
@@ -3562,6 +3564,8 @@ build_vec_init (tree base, tree maxindex, tree init,
/* Clear out INIT so that we don't get confused below. */
init = NULL_TREE;
+ /* Any elements without explicit initializers get {}. */
+ explicit_value_init_p = true;
}
else if (from_array)
{
diff --git a/gcc/testsuite/g++.dg/cpp1y/vla10.C b/gcc/testsuite/g++.dg/cpp1y/vla10.C
new file mode 100644
index 000..1c67290
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1y/vla10.C
@@ -0,0 +1,24 @@
+// PR c++/57402
+// { dg-options -std=c++1y -pedantic-errors }
+
+int i = 2;
+
+int main()
+{
+ {
+int a[i];
+a[1] = 0xbeef;
+ }
+ {
+int a[i] = { 1 };
+if (a[1] != 0)
+ __builtin_abort ();
+a[1] = 0xbeef;
+ }
+ {
+int a[i] = { };
+if (a[1] != 0)
+ __builtin_abort ();
+a[1] = 0xbeef;
+ }
+}
Re: [C++ Patch] PR 57869
On 07/09/2013 07:46 PM, Paolo Carlini wrote: Daniel noticed that, per to the resolution of DR195, we shouldn't warn in c++11 mode, not even with -pedantic, for this kind of reinterpret_cast (*). Straightforward change tested x86_64-linux, not sure if we want to do something slightly different for C++98, or something more. I think we should apply DR195 to C++98, too. We might add the warning to a new -Wconditionally-supported flag that could later warn about other conditionally-supported constructs as well. Jason
Re: Restore cr16-elf
On 07/09/2013 05:05 PM, Jan-Benedict Glaw wrote: Hi! I just tried to build cr16-elf, which fails due to a dependency problem: `gencodes' is needed to build insn-codes.h, but for building it, target.h is included in cr16-protos.h, which in turn includes insn-codes.h: [...] g++ -c -g -O2 -DIN_GCC -DCROSS_DIRECTORY_STRUCTURE -fno-exceptions -fno-rtti -fasynchronous-unwind-tables -W -Wall -Wno-narrowing -Wwrite-strings -Wcast-qual -Wmissing-format-attribute -pedantic -Wno-long-long -Wno-variadic-macros -Wno-overlength-strings -fno-common -DHAVE_CONFIG_H -DGENERATOR_FILE -I. -Ibuild -I../../../../gcc/gcc -I../../../../gcc/gcc/build -I../../../../gcc/gcc/../include -I../../../../gcc/gcc/../libcpp/include -I../../../../gcc/gcc/../libdecnumber -I../../../../gcc/gcc/../libdecnumber/dpd -I../libdecnumber -I../../../../gcc/gcc/../libbacktrace\ -o build/gencondmd.o build/gencondmd.c In file included from ../../../../gcc/gcc/config/cr16/cr16-protos.h:24:0, from ./tm_p.h:4, from build/gencondmd.c:24: ../../../../gcc/gcc/target.h:52:24: fatal error: insn-codes.h: No such file or directory compilation terminated. make[2]: *** [build/gencondmd.o] Error 1 make[2]: Leaving directory `/mnt/devel/src/linux/build/cr16-elf/gcc-stage1/gcc' make[1]: *** [all-gcc] Error 2 make[1]: Leaving directory `/mnt/devel/src/linux/build/cr16-elf/gcc-stage1' The target.h include in cr16-protos.h isn't needed, so don't include it. This allows to build for cr16-elf again. Ok to commit? 2013-07-10 Jan-Benedict Glaw jbg...@lug-owl.de gcc/ * config/cr16/cr16-protos.h: Don't include target.h. This is fine. Thanks. jeff
