Re: [Patch x86/darwin] fix PR51784 'PIC register not correctly preserved...'
On Fri, Jul 19, 2013 at 11:00 PM, Jakub Jelinek wrote:
> On Fri, Jul 19, 2013 at 04:56:47PM +0200, Uros Bizjak wrote:
>> > OK for trunk?
>>
>> Assuming that Jakub is OK with the patch, it is OK for trunk.
>
> With the line wrapping fix and Uros' suggested improvement this is ok for
> both trunk and branches.
Can you please also consider attached patch? There is no need to drag
around an UNSPEC if we don't need it anymore. Attached patch marks
instruction as deleted in this case.
The patch was tested by looking at the generated code from various
testcases in the PR.
Uros.
Index: i386.md
===
--- i386.md (revision 201091)
+++ i386.md (working copy)
@@ -16233,12 +16233,7 @@
(define_insn_and_split "nonlocal_goto_receiver"
[(unspec_volatile [(const_int 0)] UNSPECV_NLGR)]
"TARGET_MACHO && !TARGET_64BIT && flag_pic"
-{
- if (crtl->uses_pic_offset_table)
-return "#";
- else
-return ""; /* No pic reg restore needed. */
-}
+ "#"
"&& reload_completed"
[(const_int 0)]
{
@@ -16258,6 +16253,10 @@
xops[2] = gen_rtx_CONST (Pmode, tmp);
ix86_expand_binary_operator (MINUS, SImode, xops);
}
+ else
+ /* No pic reg restore needed. */
+emit_note (NOTE_INSN_DELETED);
+
DONE;
})
Re: [Patch x86/darwin] fix PR51784 'PIC register not correctly preserved...'
Hi Uros, On 21 Jul 2013, at 08:34, Uros Bizjak wrote: > Sure, I can test that - do you want me to apply it assuming it reg-tests OK on darwin & linux? (also I can amend the back ports - since I didn't have time to get to them yesterday) thanks Iain
Re: [Patch x86/darwin] fix PR51784 'PIC register not correctly preserved...'
On Sun, Jul 21, 2013 at 9:42 AM, Iain Sandoe wrote: >> > > Sure, I can test that - do you want me to apply it assuming it reg-tests OK > on darwin & linux? > (also I can amend the back ports - since I didn't have time to get to them > yesterday) Yes, please test it on darwin. The build test on linux went OK. However, the change is not effective on linux, so I have no means to properly test it by myself. If everything is OK, please also use this change in backports. The ChangeLog is attached below. 2013-07-21 Uros Bizjak * config/i386/i386.md (nonlocal_goto_receiver): Delete insn if it is not needed after split. Thanks, Uros.
Re: [Patch, Fortran] PR35862 - add input I/O rounding support - by setting the CPU rounding mode
Mikael Morin wrote: Le 17/07/2013 11:02, Tobias Burnus a écrit : Build and regtested on x86-64-gnu-linux. OK for the trunk? The fortran bits look good. Thanks for the review Mikel and David! Thanks for the patch Uros. I have now committed the patch (Rev. 201093). - Hopefully, it works on all systems (i.e. compiles and sets the round mode, when possible.) I expect the test case to fail on systems where strto(f,d,ld) doesn't honour the rounding mode - or which use the "fpu-generic.h". Hopefully, no major system is affected. For the others, we have to exclude them in the test case - unless it turns out that they are fixable without too much effort. Tobias
Re: [patch, fortran] PR 56937 - temporaries with array indices
Hi Paul, This is OK for trunk. Could you expand the comment to capture the two cases without dependency. eg. There is no dependency if the vector indices are equal or things are known to be different in a different dimension. Committed to trunk with an updated comment, as you suggested, as rev. 201094. Thanks for the review! Regards Thomas
[PATCH 1/*] Fix common typos.
Hi, This is series of typo fixing patches. They are generated with stylepp https://github.com/neleai/stylepp which makes patch generation very effective. This series should be applied in sequence to avoid reviewing duplicates. Now I exclude those directories that are upstream, see file https://github.com/neleai/stylepp/blob/master/maintained/gcc/ignore and if you want ignore additional files/directories add them to this file. Now for patch itself, situation is following: I extracted 4214 common corrections from wikipedia, in following list: https://github.com/neleai/stylepp/blob/master/maintained/dictionary_wiki I plan to add another replacements that we will frequently encounter. There is possibility to generate this list automatically from commit history, If we look when in comment commit changes word but not two adjacent words and aspell list says that changed word went from wrong to correct it is likely a typo fix. We could extract these replacement pairs and use them. I would gladly welcome somebody to volunteer to write this analysis. Then I ran script/stylepp_fix_spell which produced following 300kb patch: http://kam.mff.cuni.cz/~ondra/0001-Fix-common-typos.patch
Re: [PATCH 1/*] Fix common typos.
On Sun, 21 Jul 2013, Ondřej Bílka wrote: This is series of typo fixing patches. They are generated with stylepp https://github.com/neleai/stylepp which makes patch generation very effective. This series should be applied in sequence to avoid reviewing duplicates. Now I exclude those directories that are upstream, see file https://github.com/neleai/stylepp/blob/master/maintained/gcc/ignore and if you want ignore additional files/directories add them to this file. Now for patch itself, situation is following: I extracted 4214 common corrections from wikipedia, in following list: https://github.com/neleai/stylepp/blob/master/maintained/dictionary_wiki I plan to add another replacements that we will frequently encounter. There is possibility to generate this list automatically from commit history, If we look when in comment commit changes word but not two adjacent words and aspell list says that changed word went from wrong to correct it is likely a typo fix. We could extract these replacement pairs and use them. I would gladly welcome somebody to volunteer to write this analysis. Then I ran script/stylepp_fix_spell which produced following 300kb patch: http://kam.mff.cuni.cz/~ondra/0001-Fix-common-typos.patch There are still some wrong fixes, humans really need to check each of them (for instance throught -> thought instead of through). I think I saw a few overlong lines (more than 80 chars on a line, counting a tab as 8) after replacement. I don't know if capitalizing all instances of "ok" is really worth it... -- Marc Glisse
Re: [PATCH 1/*] Fix common typos.
I do not believe such fixes in testsuites make sense. They are definitely inappropriate in testsuite input data (as opposed to comments) without specific rationale in the patch posting for why the change is OK. Changes to gcc/go/gofrontend/ and libgo/ have their own rules as those directories are maintained elsewhere. Please send them separately. Fortran has its own mailing list. Please send those changes separately (to gcc-patches@ and fortran@). Java has its own mailing list. Please send those separately (to gcc-patches@ and java-patches@). libstdc++ has its own mailing list. Please send those separately (to gcc-patches@ and libstdc++@). lwg-closed.html and lwg-defects.html are externally maintained and patches to those would need to go upstream. In general, splitting up by directory is a good idea even if there isn't a separate mailing list, to keep the patches down to a size easy for humans to review quickly, and given that different people maintain different files and directories. soft-fp comes from glibc. Please send patches to it to glibc in the first instance. In any case, make sure the patches, after splitting up by directory, are small enough to post directly to the list, and post them rather than URLs; split up further if necessary (really I'd suggest keeping the amount of such changes you have pending review in total down to about 2000 lines - wait for the pending patches to be reviewed before sending more). -- Joseph S. Myers jos...@codesourcery.com
Re: [Patch, Fortran] PR35862 - add input I/O rounding support - by setting the CPU rounding mode
Tobias Burnus wrote:
I have now committed the patch (Rev. 201093).
I missed the attached patch, which fixes the build. (Rev. 201095)
Tobias
Index: libgfortran/ChangeLog
===
--- libgfortran/ChangeLog (Revision 201094)
+++ libgfortran/ChangeLog (Arbeitskopie)
@@ -1,4 +1,10 @@
2013-07-21 Tobias Burnus
+
+ PR fortran/35862
+ * config/fpu-387.h (set_fpu_rounding_mode,
+ get_fpu_rounding_mode): Add missing _ to fix build.
+
+2013-07-21 Tobias Burnus
Uros Bizjak
PR fortran/35862
Index: libgfortran/config/fpu-387.h
===
--- libgfortran/config/fpu-387.h (Revision 201094)
+++ libgfortran/config/fpu-387.h (Arbeitskopie)
@@ -202,7 +202,7 @@ set_fpu_rounding_mode (int round)
__asm__ __volatile__ ("fnstcw\t%0" : "=m" (cw));
- cw &= ~FPU_RC_MASK;
+ cw &= ~_FPU_RC_MASK;
cw |= round_mode;
__asm__ __volatile__ ("fldcw\t%0" : : "m" (cw));
@@ -214,7 +214,7 @@ set_fpu_rounding_mode (int round)
__asm__ __volatile__ ("%vstmxcsr\t%0" : "=m" (cw_sse));
/* The SSE round control bits are shifted by 3 bits. */
- cw_sse &= ~(FPU_RC_MASK << 3);
+ cw_sse &= ~(_FPU_RC_MASK << 3);
cw_sse |= round_mode << 3;
__asm__ __volatile__ ("%vldmxcsr\t%0" : : "m" (cw_sse));
@@ -228,7 +228,7 @@ get_fpu_rounding_mode (void)
__asm__ __volatile__ ("fnstcw\t%0" : "=m" (cw));
- cw &= FPU_RC_MASK;
+ cw &= _FPU_RC_MASK;
switch (cw)
{
Re: [PATCH] FPU IEEE 754 for MIPS r5900
Hello Richard, > "Jürgen Urban" writes: > >> "Jürgen Urban" writes: > >> >> "Jürgen Urban" 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. I wasn't sure what sticky bits means. Now I got the information what the purpose of the sticky bits are. After reading the definition and testing, I think the rounding is not sign dependent. Ignoring sticky bits are only causing rounding towards 0. > > 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? The kernel disabled the FPU, because otherwise code from the official Fedora 12 release crashs. The FPU is emulated by the kernel. I needed to recompile it to enable the FPU. Now I used the old GCC to compile the kernel. > >> > 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 ar
Re: RFC: Add of type-demotion pass
Marc Glisse wrote: >On Fri, 19 Jul 2013, Jeff Law wrote: > >> It's also worth noting that fold-const.c also does some type >> hoisting/sinking. > >fold-const.c does everything ;-) > >> Ideally that code should just be going away. > >Like most of the code from fold-const.c that isn't about folding >constants, I guess, once it has a replacement at the gimple level. > > 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, >>> >>> Could you explain in what case? I guess my trouble understanding >this is >>> the same as in the next question, and I am missing a fundamental >point... >> Anytime you hoist/sink a cast, you're moving it across an operation >-- which >> can expose it as a redundant cast. >> >> Let's say you start with >> >> A = (T) x1; >> B = (T) x2; >> R = A & B; >> >> >> And sink the cast after the operation like this: >> >> C = x1 & x2; >> R = (T) C; >> >> We may find that that cast of C to type T is redundant. Similar >cases can be >> found when we hoist the cast across the operation. I saw this kind >of >> situation occur regularly when I was looking at Kai's >hoisting/sinking >> patches. > >IIRC the question here was how forwprop could miss opportunities that a > >backward (reverse dominance) walk finds, and I don't see that in this >example. If the cast is redundant, forwprop is just as likely to detect > >it. > >> Now I believe one of Kai's goals is to allow our various pattern >based >> folders to work better by not having to account for casting >operations as >> often in sequences of statements we want to fold. > >Ah. I thought it was mostly so operations would be performed in a >smaller, >hopefully cheaper mode. Simplifying combiner patterns would be nice if >that worked. > 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. >>> >>> It looks to me like a gimple version of fold, except that since it >is >>> gimple, basic operations are an order of magnitude more complicated. >But >>> I don't really see why that would make it an unmaintainable mess, >giant >>> switches are not that bad. >> It's certainly moving towards a gimple version of fold and special >casing >> everything as we convert from fold and to gimple_fold (or whatever we >call >> it) is going to result in a horrid mess. >> >> I find myself thinking that at a high level we need to split out the >forward >> and backward propagation bits into distinct passes. > >They are already mostly split (2 separate loops in >ssa_forward_propagate_and_combine), so that wouldn't be hard. > >> The backward propagation >> bits are just a tree combiner and the idioms used to follow the >backward >> chains to create more complex trees and fold them need to be reusable > >> components. It's totally silly (and ultimately unmaintainable) that >each >> transformation is open-coding the walking of the use-def chain and >> simplification step. Yeah - ideally the pattern matching would be abstract enough that it works on trees, gimple and rtl ... Using a domain specific language like Haskell would get you pattern combining for free. But I suppose with c++ you can come close enough to at least support tree and gimple ssa. >It isn't completely open-coded. get_prop_source_stmt, >can_propagate_from, >defcodefor_name, etc make walking the use-def chain not so bad. > >Usually at this point Richard refers to Andrew's work on a gimple >combiner... That or the gimple folding interface. Note the idea behind both should be to make the combined accessible from random passes just like we use fold_build now. Richard.
Re: [PATCH 1/*] Fix common typos.
On Sun, Jul 21, 2013 at 04:32:04PM +0200, Ondřej Bílka wrote:
> Hi,
>
> This is series of typo fixing patches. They are generated with stylepp
> https://github.com/neleai/stylepp
> which makes patch generation very effective.
>
> This series should be applied in sequence to avoid reviewing duplicates.
>
> Now I exclude those directories that are upstream, see file
> https://github.com/neleai/stylepp/blob/master/maintained/gcc/ignore
> and if you want ignore additional files/directories add them to this
> file.
>
> Now for patch itself, situation is following:
> I extracted 4214 common corrections from wikipedia, in following list:
> https://github.com/neleai/stylepp/blob/master/maintained/dictionary_wiki
> I plan to add another replacements that we will frequently encounter.
>
> There is possibility to generate this list automatically from commit
> history, If we look when in comment commit changes word but not two
> adjacent words and aspell list says that changed word went from wrong to
> correct it is likely a typo fix. We could extract these
> replacement pairs and use them. I would gladly welcome somebody to
> volunteer to write this analysis.
>
> Then I ran script/stylepp_fix_spell which produced following 300kb patch:
>
> http://kam.mff.cuni.cz/~ondra/0001-Fix-common-typos.patch
A few comments:
diff --git a/gcc/ada/gcc-interface/utils2.c b/gcc/ada/gcc-interface/utils2.c
index 3f39a43..7f7f6af 100644
--- a/gcc/ada/gcc-interface/utils2.c
+++ b/gcc/ada/gcc-interface/utils2.c
@@ -1902,7 +1902,7 @@ build_simple_component_ref (tree record_variable, tree
component,
{
tree new_field;
- /* First loop thru normal components. */
+ /* First loop through normal components. */
"thru" is not a typo.
- then all auto increment forms are ok. */
+ then all auto increment forms are OK. */
The patch would be much smaller if we'd keep "ok"s.
diff --git a/gcc/config/tilepro/tilepro.c b/gcc/config/tilepro/tilepro.c
index 59618e4..1d9adbd 100644
--- a/gcc/config/tilepro/tilepro.c
+++ b/gcc/config/tilepro/tilepro.c
@@ -1365,7 +1365,7 @@ expand_set_cint32 (rtx dest_reg, rtx src_val)
three_wide_only))
{
/* 0xA500 becomes:
-movei temp, 0xFFA5
+movie temp, 0xFFA5
That doesn't seem to be correct.
shli dest, temp, 8 */
emit_move_insn (dest_reg,
gen_rtx_ASHIFT (SImode, temp,
@@ -1377,7 +1377,7 @@ expand_set_cint32 (rtx dest_reg, rtx src_val)
three_wide_only))
{
/* 0x7FFF becomes:
-movei temp, -2
+movie temp, -2
Likewise.
shri dest, temp, 1 */
emit_move_insn (dest_reg,
gen_rtx_LSHIFTRT (SImode, temp,
@@ -1393,7 +1393,7 @@ expand_set_cint32 (rtx dest_reg, rtx src_val)
if (expand_set_cint32_one_inst (temp, r, three_wide_only))
{
/* 0xFFA5 becomes:
-movei temp, 0xFFA5
+movie temp, 0xFFA5
Likewise.
rli dest, temp, 16 */
emit_move_insn (dest_reg,
gen_rtx_ROTATE (SImode, temp, GEN_INT (count)));
diff --git a/gcc/ipa.c b/gcc/ipa.c
index 7c0d495..d44cf38 100644
--- a/gcc/ipa.c
+++ b/gcc/ipa.c
@@ -548,7 +548,7 @@ static bool
comdat_can_be_unshared_p_1 (symtab_node node)
{
/* When address is taken, we don't know if equality comparison won't
- break eventaully. Exception are virutal functions and vtables, where
+ break eventaully. Exception are virtual functions and vtables, where
"eventually"
diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index 2787d67..b0dad7d 100644
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,64 @@
+2013-07-21 OndÅej BÃlka
+
+ * c-c++-common/pr41779.c: Fix typos.
+ * c-c++-common/raw-string-13.c: Likewise.
+ * c-c++-common/raw-string-14.c: Likewise.
+ * c-c++-common/raw-string-15.c: Likewise.
+ * c-c++-common/raw-string-16.c: Likewise.
+ * c-c++-common/raw-string-2.c: Likewise.
+ * ChangeLog: Likewise.
ChangeLog in a ChangeLog?
diff --git a/gcc/testsuite/c-c++-common/pr41779.c
b/gcc/testsuite/c-c++-common/pr41779.c
index 80c8e6b..f80412c 100644
--- a/gcc/testsuite/c-c++-common/pr41779.c
+++ b/gcc/testsuite/c-c++-common/pr41779.c
@@ -1,4 +1,4 @@
-/* PR41779: Wconversion cannot see throught real*integer promotions. */
+/* PR41779: Wconversion cannot see thought real*integer promotions. */
This change is not ok (eh, OK), it should've been "through".
diff --git a/libiberty/regex.c b/libiberty/regex.c
index 17091ce..8a2dd41 100644
--- a/libiberty/regex.c
+++ b/libiberty/regex.c
@@ -3396,7 +3396,7 @@ PREFIX(regex_compile) (const char *ARG_PREFIX(pattern),
class. */
PATFETCH (c);
-
Re: [PATCH 1/*] Fix common typos.
On Jul 21, 2013, at 7:32 AM, Ondřej Bílka wrote: > + * c-c++-common/raw-string-13.c: Likewise. > > + * c-c++-common/raw-string-14.c: Likewise. > > + * c-c++-common/raw-string-15.c: Likewise. > > + * c-c++-common/raw-string-16.c: Likewise. > > + * c-c++-common/raw-string-2.c: Likewise. I didn't see these in the patch.
Re: [PATCH 1/*] Fix common typos.
On Sun, Jul 21, 2013 at 05:37:23PM +0200, Marek Polacek wrote:
>
> A few comments:
>
snip
> diff --git a/gcc/ipa.c b/gcc/ipa.c
> index 7c0d495..d44cf38 100644
> --- a/gcc/ipa.c
> +++ b/gcc/ipa.c
> @@ -548,7 +548,7 @@ static bool
> comdat_can_be_unshared_p_1 (symtab_node node)
> {
>/* When address is taken, we don't know if equality comparison won't
> - break eventaully. Exception are virutal functions and vtables, where
> + break eventaully. Exception are virtual functions and vtables, where
>
> "eventually"
>
It is new misspell so it should not be part of this patch but next one.
When you encounter them could you write list with form:
eventaully eventually
...
to make integration easier?
> diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
> index 2787d67..b0dad7d 100644
> --- a/gcc/testsuite/ChangeLog
> +++ b/gcc/testsuite/ChangeLog
> @@ -1,3 +1,64 @@
> +2013-07-21 OndÅej BÃlka
> +
> + * c-c++-common/pr41779.c: Fix typos.
> + * c-c++-common/raw-string-13.c: Likewise.
> + * c-c++-common/raw-string-14.c: Likewise.
> + * c-c++-common/raw-string-15.c: Likewise.
> + * c-c++-common/raw-string-16.c: Likewise.
> + * c-c++-common/raw-string-2.c: Likewise.
> + * ChangeLog: Likewise.
>
> ChangeLog in a ChangeLog?
>
yeah, I ran generating script with previous commit so it was wrong.
> diff --git a/libsanitizer/ChangeLog b/libsanitizer/ChangeLog
> index d2c80b3..a6c7d5f 100644
> --- a/libsanitizer/ChangeLog
> +++ b/libsanitizer/ChangeLog
> @@ -1,3 +1,7 @@
> +2013-07-21 OndÅej BÃlka
> +
> + * tsan/tsan_interface_atomic.cc: Fix typos.
>
> I don't think we should make changes to libsanitizer/*.
>
On Sun, Jul 21, 2013 at 04:32:04PM +0200, Ondřej Bílka wrote:
>> Now I exclude those directories that are upstream, see file
>> https://github.com/neleai/stylepp/blob/master/maintained/gcc/ignore
>> and if you want ignore additional files/directories add them to this
>> file.
Re: [PATCH 1/*] Fix common typos.
On Sun, Jul 21, 2013 at 09:09:37AM -0700, Mike Stump wrote: > On Jul 21, 2013, at 7:32 AM, Ondřej Bílka wrote: > > + * c-c++-common/raw-string-13.c: Likewise. > > > > + * c-c++-common/raw-string-14.c: Likewise. > > > > + * c-c++-common/raw-string-15.c: Likewise. > > > > + * c-c++-common/raw-string-16.c: Likewise. > > > > + * c-c++-common/raw-string-2.c: Likewise. > > > I didn't see these in the patch. because I accidentaly generated them with previous commit, will fix in next version.
Re: [PATCH 1/*] Fix common typos.
On Jul 21, 2013, at 7:32 AM, Ondřej Bílka wrote: > This is series of typo fixing patches. They are generated with stylepp > https://github.com/neleai/stylepp > which makes patch generation very effective. I've checked in most changes to Objective-C things and test suite things after reviewing all those changes. I agreed with most of the work, except ok -> OK. We don't need to scream in the source, and ok I feel is a fine english word, despite what an expert might think (they would burn us (me) at the grammar stake. They merely trail us in language adoption. :-) One part I will throw out here, in gcc/testsuite/gcc.target/sh/pr54760-4.c: - and its use when a function call is inbetween, when GBR is a call used + and its use when a function call is between, when GBR is a call used I think this should be: - and its use when a function call is inbetween, when GBR is a call used + and its use when a function call is in-between, when GBR is a call used If someone wants to try and counter this, please, I'd favor an expert that can clarify why it would be preferable.
Re: [PATCH 1/*] Fix common typos.
On Jul 21, 2013, at 8:37 AM, Marek Polacek wrote:
> diff --git a/gcc/ada/gcc-interface/utils2.c b/gcc/ada/gcc-interface/utils2.c
> index 3f39a43..7f7f6af 100644
> --- a/gcc/ada/gcc-interface/utils2.c
> +++ b/gcc/ada/gcc-interface/utils2.c
> @@ -1902,7 +1902,7 @@ build_simple_component_ref (tree record_variable, tree
> component,
> {
> tree new_field;
>
> - /* First loop thru normal components. */
> + /* First loop through normal components. */
>
> "thru" is not a typo.
Yes, it is. Note, this _is_ a value judgement. The source code is not a sign,
and we do no accept the desire by some people to make thru standard, except for
it's usage in space constrained places, like signs and headlines and inform
writing. Drive-thru is fine.
> - then all auto increment forms are ok. */
> + then all auto increment forms are OK. */
>
> The patch would be much smaller if we'd keep "ok"s.
I don't care about size, but, I do prefer ok
> diff --git a/gcc/testsuite/c-c++-common/pr41779.c
> b/gcc/testsuite/c-c++-common/pr41779.c
> index 80c8e6b..f80412c 100644
> --- a/gcc/testsuite/c-c++-common/pr41779.c
> +++ b/gcc/testsuite/c-c++-common/pr41779.c
> @@ -1,4 +1,4 @@
> -/* PR41779: Wconversion cannot see throught real*integer promotions. */
> +/* PR41779: Wconversion cannot see thought real*integer promotions. */
>
> This change is not ok (eh, OK), it should've been "through".
Thanks.
Re: [PATCH 1/*] Fix common typos.
I've applied these fixes, to ensure that they are not corrected incorrectly,
thanks for the corrections.
On Jul 21, 2013, at 8:37 AM, Marek Polacek wrote:
> diff --git a/gcc/ipa.c b/gcc/ipa.c
> index 7c0d495..d44cf38 100644
> --- a/gcc/ipa.c
> +++ b/gcc/ipa.c
> @@ -548,7 +548,7 @@ static bool
> comdat_can_be_unshared_p_1 (symtab_node node)
> {
> /* When address is taken, we don't know if equality comparison won't
> - break eventaully. Exception are virutal functions and vtables, where
> + break eventaully. Exception are virtual functions and vtables, where
>
> "eventually"
Fixed.
> diff --git a/libiberty/regex.c b/libiberty/regex.c
> index 17091ce..8a2dd41 100644
> --- a/libiberty/regex.c
> +++ b/libiberty/regex.c
> @@ -3396,7 +3396,7 @@ PREFIX(regex_compile) (const char *ARG_PREFIX(pattern),
> class. */
> PATFETCH (c);
>
> - /* Now we have to go throught the whole table
> + /* Now we have to go thought the whole table
>
> Again, throught -> through.
Fixed.
> diff --git a/libstdc++-v3/testsuite/ext/pb_ds/example/hash_resize.cc
> b/libstdc++-v3/testsuite/ext/pb_ds/example/hash_resize.cc
> index 1afb9b9..a5e026e 100644
> --- a/libstdc++-v3/testsuite/ext/pb_ds/example/hash_resize.cc
> +++ b/libstdc++-v3/testsuite/ext/pb_ds/example/hash_resize.cc
> @@ -36,7 +36,7 @@
>
> /**
> * This example shows how to externally manipulate the size of a hash-based
> - * container object throught its resize-policy object.
> + * container object thought its resize-policy object.
>
> Again, throught -> through.
Fixed.
Re: [PATCH 1/*] Fix common typos.
On Jul 21, 2013, at 7:32 AM, Ondřej Bílka wrote: > This is series of typo fixing patches. I've reviewed and applied the gcc/doc changes that were trivial. The only patches not applied were the ok->OK patches.
Re: [PATCH 1/*] Fix common typos.
On Sun, Jul 21, 2013 at 04:44:40PM +0200, Marc Glisse wrote: > On Sun, 21 Jul 2013, Ondřej Bílka wrote: > > >Then I ran script/stylepp_fix_spell which produced following 300kb patch: > > > >http://kam.mff.cuni.cz/~ondra/0001-Fix-common-typos.patch > > There are still some wrong fixes, humans really need to check each > of them (for instance throught -> thought instead of through). I > think I saw a few overlong lines (more than 80 chars on a line, > counting a tab as 8) after replacement. I try to keep these issues separate. Dealing with long lines is one of these. Now I added another tool to stylepp. If you add stylepp/script to PATH then you can run stylepp_long_line --hook Which will open vim on lines with long lines. A hook flag is for opening only files that were changed since last commit (only git for now). > I don't know if capitalizing > all instances of "ok" is really worth it... > Well both are valid, according to wikipedia OK is better because it ok is harder to spot when you glance through code. http://en.wikipedia.org/wiki/Okay Problem here is not ok itself but that diffs in patch are organized in mostly random order. If I could sort hunks such that typos come in alphabetic order it would be easier to review repeated corrections. I am not sure how to get this. > -- > Marc Glisse
Re: [PATCH 1/*] Fix common typos.
On Jul 21, 2013, at 7:32 AM, Ondřej Bílka wrote: > This is series of typo fixing patches. I reviewed and applied all the trivial patches in gcc/config. Excluded were ok->OK, and /* My current feeling is that r14 should go to the end and maybe even r12. - It seems like the overhead of store/load that will occur since we cant + It seems like the overhead of store/load that will occur since we cannot pair anything up with r14 will be higher than the advantage of smaller as I wasn't sure if I liked can't or cannot. The choice made the change non-trivial. I know, sounds silly, but, I was trying to be very conservative in what I find trivial.
Re: [Patch] Partially implement regex_search
On 18 July 2013 11:06, Tim Shen wrote: > > These will be fixed when commit to SVN. OK. The rest of the patch is fine - would you like me to commit it for you, with those fixes?
Re: [PATCH 1/*] Fix common typos.
On Jul 21, 2013, at 7:32 AM, Ondřej Bílka wrote: > This is series of typo fixing patches. I checked in all the changes to gcc/cp that were trivial.
Re: [PATCH 1/*] Fix common typos.
Ondřej Bílka wrote:
This is series of typo fixing patches. They are generated with stylepp
https://github.com/neleai/stylepp
which makes patch generation very effective.
[...]
Then I ran script/stylepp_fix_spell which produced following 300kb patch:
http://kam.mff.cuni.cz/~ondra/0001-Fix-common-typos.patch
I committed (Rev. 201107) the Fortran patches I found in the huge patch
- see attachment. As there was still a debate about ok vs. OK, I
excluded those.
Tobias
Index: gcc/fortran/ChangeLog
===
--- gcc/fortran/ChangeLog (Revision 201105)
+++ gcc/fortran/ChangeLog (Arbeitskopie)
@@ -1,3 +1,8 @@
+2013-07-21 OndÅej BÃlka
+
+ * trans-decl.c: Fix comment typos.
+ * trans-expr.c: Ditto.
+
2013-07-21 Thomas Koenig
PR fortran/56937
Index: gcc/fortran/trans-decl.c
===
--- gcc/fortran/trans-decl.c (Revision 201105)
+++ gcc/fortran/trans-decl.c (Arbeitskopie)
@@ -2157,7 +2157,7 @@ create_function_arglist (gfc_symbol * sym)
}
}
/* For noncharacter scalar intrinsic types, VALUE passes the value,
- hence, the optional status cannot be transfered via a NULL pointer.
+ hence, the optional status cannot be transferred via a NULL pointer.
Thus, we will use a hidden argument in that case. */
else if (f->sym->attr.optional && f->sym->attr.value
&& !f->sym->attr.dimension && f->sym->ts.type != BT_CLASS
Index: gcc/fortran/trans-expr.c
===
--- gcc/fortran/trans-expr.c (Revision 201105)
+++ gcc/fortran/trans-expr.c (Arbeitskopie)
@@ -1235,7 +1235,7 @@ gfc_conv_expr_present (gfc_symbol * sym)
as actual argument to denote absent dummies. For array descriptors,
we thus also need to check the array descriptor. For BT_CLASS, it
can also occur for scalars and F2003 due to type->class wrapping and
- class->class wrapping. Note futher that BT_CLASS always uses an
+ class->class wrapping. Note further that BT_CLASS always uses an
array descriptor for arrays, also for explicit-shape/assumed-size. */
if (!sym->attr.allocatable
@@ -2829,7 +2829,7 @@ gfc_build_compare_string (tree len1, tree str1, tr
/* We can compare via memcpy if the strings are known to be equal
in length and they are
- kind=1
- - kind=4 and the comparision is for (in)equality. */
+ - kind=4 and the comparison is for (in)equality. */
if (INTEGER_CST_P (len1) && INTEGER_CST_P (len2)
&& tree_int_cst_equal (len1, len2)
Index: libgfortran/ChangeLog
===
--- libgfortran/ChangeLog (Revision 201105)
+++ libgfortran/ChangeLog (Arbeitskopie)
@@ -1,3 +1,7 @@
+2013-07-21 OndÅej BÃlka
+
+ * io/transfer.c: Fix comment typos.
+
2013-07-21 Tobias Burnus
PR fortran/35862
Index: libgfortran/io/transfer.c
===
--- libgfortran/io/transfer.c (Revision 201105)
+++ libgfortran/io/transfer.c (Arbeitskopie)
@@ -233,7 +233,7 @@ read_sf_internal (st_parameter_dt *dtp, int * leng
{
*length = 0;
/* Just return something that isn't a NULL pointer, otherwise the
- caller thinks an error occured. */
+ caller thinks an error occurred. */
return (char*) empty_string;
}
@@ -292,7 +292,7 @@ read_sf (st_parameter_dt *dtp, int * length)
{
*length = 0;
/* Just return something that isn't a NULL pointer, otherwise the
- caller thinks an error occured. */
+ caller thinks an error occurred. */
return (char*) empty_string;
}
@@ -512,7 +512,7 @@ read_block_form4 (st_parameter_dt *dtp, int * nbyt
{
*nbytes = 0;
/* Just return something that isn't a NULL pointer, otherwise the
- caller thinks an error occured. */
+ caller thinks an error occurred. */
return empty_string;
}
Re: [PATCH 1/*] Fix common typos.
On Jul 21, 2013, at 7:32 AM, Ondřej Bílka wrote:
> This is series of typo fixing patches.
I reviewed and checked in all that changes to gcc/tree\* that were trivial. I
left out ok->OK, and
--- a/gcc/tree-ssa-pre.c
+++ b/gcc/tree-ssa-pre.c
@@ -1894,7 +1894,7 @@ value_dies_in_block_x (pre_expr expr, basic_block block)
/* A memory expression {e, VUSE} dies in the block if there is a
statement that may clobber e. If, starting statement walk from the
top of the basic block, a statement uses VUSE there can be no kill
- inbetween that use and the original statement that loaded {e, VUSE},
+ between that use and the original statement that loaded {e, VUSE},
so we can stop walking. */
ref.base = NULL_TREE;
for (gsi = gsi_start_bb (block); !gsi_end_p (gsi); gsi_next (&gsi))
which I didn't find trivial (being very conservative here). For the first, I
prefer ok (as do many contributors).
Re: [PATCH 1/*] Fix common typos.
On Jul 21, 2013, at 7:32 AM, Ondřej Bílka wrote: > This is series of typo fixing patches. I checked in the fixes to gcc/[l-t]* that were trivial. (ok->OK being the usual exclusion).
Re: [ubsan] Add libcall arguments
On Sat, Jul 20, 2013 at 08:17:34PM -0400, Jason Merrill wrote:
> On 07/20/2013 10:27 AM, Jakub Jelinek wrote:
> >So, the middle-end can only use
> >some other type, say build_nonstandard_integer_type (POINTER_SIZE, 1);
> >which often will be the same type as uintptr_type_node, but perhaps not on
> >all targets. It could be worth to create such a type in tree.c and stick it
> >into global trees and then just use, but right now all the uses are just in
> >asan.c and ubsan.c.
>
> Let's do that; I expect it will be useful in other situations, too.
Ok, here's a patch for that. Does the name sound reasonable?
2013-07-21 Marek Polacek
* tree.h (enum tree_index): Add TI_POINTER_SIZED_TYPE.
(pointer_sized_type_node): Define.
* tree.c (build_common_tree_nodes): Initialize
pointer_sized_type_node.
* ubsan.c (ubsan_encode_value): Use pointer_sized_type_node instead
calling uptr_type.
(uptr_type): Remove function.
* asan.c (asan_global_struct): Use pointer_sized_type_node instead
calling build_nonstandard_integer_type.
(initialize_sanitizer_builtins): Likewise.
(asan_finish_file): Likewise.
--- gcc/tree.c.mp 2013-07-21 19:54:35.416986756 +0200
+++ gcc/tree.c 2013-07-21 19:56:58.347562787 +0200
@@ -9638,6 +9638,8 @@ build_common_tree_nodes (bool signed_cha
= build_pointer_type (build_type_variant (void_type_node, 1, 0));
fileptr_type_node = ptr_type_node;
+ pointer_sized_type_node = build_nonstandard_integer_type (POINTER_SIZE, 1);
+
float_type_node = make_node (REAL_TYPE);
TYPE_PRECISION (float_type_node) = FLOAT_TYPE_SIZE;
layout_type (float_type_node);
--- gcc/ubsan.c.mp 2013-07-21 20:04:59.469653493 +0200
+++ gcc/ubsan.c 2013-07-21 20:07:00.227178083 +0200
@@ -123,14 +123,6 @@ ubsan_typedesc_new (tree type, tree decl
return desc;
}
-/* Build the ubsan uptr type. */
-
-static tree
-uptr_type (void)
-{
- return build_nonstandard_integer_type (POINTER_SIZE, 1);
-}
-
/* Helper routine, which encodes a value in the uptr type.
Arguments with precision <= POINTER_SIZE are passed directly,
the rest is passed by reference. T is a value we are to encode. */
@@ -143,7 +135,7 @@ ubsan_encode_value (tree t)
{
case INTEGER_TYPE:
if (TYPE_PRECISION (type) <= POINTER_SIZE)
- return fold_build1 (NOP_EXPR, uptr_type (), t);
+ return fold_build1 (NOP_EXPR, pointer_sized_type_node, t);
else
return build_fold_addr_expr (t);
case REAL_TYPE:
@@ -153,7 +145,7 @@ ubsan_encode_value (tree t)
{
tree itype = build_nonstandard_integer_type (bitsize, true);
t = fold_build1 (VIEW_CONVERT_EXPR, itype, t);
- return fold_convert (uptr_type (), t);
+ return fold_convert (pointer_sized_type_node, t);
}
else
{
--- gcc/tree.h.mp 2013-07-21 19:54:35.441986868 +0200
+++ gcc/tree.h 2013-07-21 19:56:05.128353854 +0200
@@ -4227,6 +4227,7 @@ enum tree_index
TI_VA_LIST_FPR_COUNTER_FIELD,
TI_BOOLEAN_TYPE,
TI_FILEPTR_TYPE,
+ TI_POINTER_SIZED_TYPE,
TI_DFLOAT32_TYPE,
TI_DFLOAT64_TYPE,
@@ -4383,6 +4384,7 @@ extern GTY(()) tree global_trees[TI_MAX]
#define va_list_fpr_counter_field
global_trees[TI_VA_LIST_FPR_COUNTER_FIELD]
/* The C type `FILE *'. */
#define fileptr_type_node global_trees[TI_FILEPTR_TYPE]
+#define pointer_sized_type_node
global_trees[TI_POINTER_SIZED_TYPE]
#define boolean_type_node global_trees[TI_BOOLEAN_TYPE]
#define boolean_false_node global_trees[TI_BOOLEAN_FALSE]
--- gcc/asan.c.mp 2013-07-21 20:07:15.013237456 +0200
+++ gcc/asan.c 2013-07-21 20:16:10.929376734 +0200
@@ -1954,7 +1954,7 @@ asan_global_struct (void)
= build_decl (UNKNOWN_LOCATION, FIELD_DECL,
get_identifier (field_names[i]),
(i == 0 || i == 3) ? const_ptr_type_node
- : build_nonstandard_integer_type (POINTER_SIZE, 1));
+ : pointer_sized_type_node);
DECL_CONTEXT (fields[i]) = ret;
if (i)
DECL_CHAIN (fields[i - 1]) = fields[i];
@@ -2039,8 +2039,7 @@ initialize_sanitizer_builtins (void)
ptr_type_node, ptr_type_node, NULL_TREE);
tree BT_FN_VOID_PTR_PTRMODE
= build_function_type_list (void_type_node, ptr_type_node,
- build_nonstandard_integer_type (POINTER_SIZE,
- 1), NULL_TREE);
+ pointer_sized_type_node, NULL_TREE);
tree BT_FN_VOID_INT
= build_function_type_list (void_type_node, integer_type_node, NULL_TREE);
tree BT_FN_BOOL_VPTR_PTR_IX_INT_INT[5];
@@ -2197,7 +2196,6 @@ asan_finish_file (void)
if (gcount)
{
tree type = asan_global_struct (), var, ctor;
- tree uptr = build_nonstandard_integer_type (POINTER_SIZ
Re: [PATCH 1/*] Fix common typos.
On Jul 21, 2013, at 7:32 AM, Ondřej Bílka wrote: > gcc/c/c-decl.c | 4 +- > > gcc/c/c-objc-common.c | 4 +- > > gcc/c/c-typeck.c | 4 +- ENOPATCH.
Re: [PATCH 1/*] Fix common typos.
On Jul 21, 2013, at 7:32 AM, Ondřej Bílka wrote: > This is series of typo fixing patches. I reviewed gcc/[a-i]* and checked in those I thought were trivial.
Re: [PATCH 1/*] Fix common typos.
On Jul 21, 2013, at 11:31 AM, Mike Stump wrote: > On Jul 21, 2013, at 7:32 AM, Ondřej Bílka wrote: >> This is series of typo fixing patches. > > I reviewed gcc/[a-i]* and checked in those I thought were trivial. So, I checked in the potentially controversial thru -> through in gcc/ada. I'll let the ada folks contemplate this and decide if they do in fact prefer thru in gcc/ada. I'll just note that in a googlefight of gcc/*.[ch] and gcc/doc/*.texi, through does win out except for words like fall-thru and abbreviations in function names, in which it is entirely appropriate. thru occurs 0 times, through occurs 1584 times. Since I was doing google fights in the code: up:gcc mrs$ grep '\' *.[ch] doc/*.texi | grep -v _OK | wc -l 95 up:gcc mrs$ grep '\' *.[ch] doc/*.texi | grep -v _ok | wc -l 286 :-) I thought I would share.
Re: [patch] proposed fix for libstdc++/54352
On 16 June 2013 15:35, Jonathan Wakely wrote: > In order to conform to [thread.condition.condvarany]/5 and fix this > PR we need an ABI change to std::condition_variable_any, so that its > internal mutex can outlive the condition_variable_any while there are > threads blocked on the mutex, which this patch does by using a > shared_ptr to hold the mutex and making waiting threads share > ownership of that mutex. > > The ABI change is handled similarly to the recent chrono::steady_clock > changes, using an inline namespace for the new versions and continuing > to export the old version from the library for compatibility with old > code. > > Any thoughts or suggestions on improving this before I commit it to trunk? > > I can no longer reproduce the valgrind errors, except by hacking in > changes to std::mutex to cause races, so I haven't been able to write > a test case that fails without this change, but I believe it's still > necessary for correctness. > > PR libstdc++/54352 > * include/std/condition_variable (condition_variable_any): Move into > inline namespace _V2 and replace mutex member with shared_ptr. > * src/c++11/condition_variable.cc (condition_variable_any): Move > definitions to ... > * src/c++11/compatibility-condvar.cc (condition_variable_any): Here. > * src/Makefile.am: Add new source file. > * src/Makefile.in: Regenerate. I've just committed this to trunk.
Re: [PATCH 1/*] Fix common typos.
On Sun, 21 Jul 2013, Mike Stump wrote: > I've reviewed and applied the gcc/doc changes that were trivial. The > only patches not applied were the ok->OK patches. *For formal documentation* such as gcc/doc, I think changing ok->OK is appropriate; the formal documentation should be more conservative about English style than code comments need to be. -- Joseph S. Myers jos...@codesourcery.com
Re: [PATCH 1/*] Fix common typos.
On Sun, 21 Jul 2013, Mike Stump wrote: > > diff --git a/gcc/testsuite/c-c++-common/pr41779.c > > b/gcc/testsuite/c-c++-common/pr41779.c > > index 80c8e6b..f80412c 100644 > > --- a/gcc/testsuite/c-c++-common/pr41779.c > > +++ b/gcc/testsuite/c-c++-common/pr41779.c > > @@ -1,4 +1,4 @@ > > -/* PR41779: Wconversion cannot see throught real*integer promotions. */ > > +/* PR41779: Wconversion cannot see thought real*integer promotions. */ > > > > This change is not ok (eh, OK), it should've been "through". The checked-in version of this file is still wrong, it now has "though" which should be "through". -- Joseph S. Myers jos...@codesourcery.com
Re: [PATCH 1/*] Fix common typos.
On Sun, 21 Jul 2013, Mike Stump wrote:
> > diff --git a/libiberty/regex.c b/libiberty/regex.c
> > index 17091ce..8a2dd41 100644
> > --- a/libiberty/regex.c
> > +++ b/libiberty/regex.c
> > @@ -3396,7 +3396,7 @@ PREFIX(regex_compile) (const char
> > *ARG_PREFIX(pattern),
> >class. */
> > PATFETCH (c);
> >
> > - /* Now we have to go throught the whole table
> > + /* Now we have to go thought the whole table
> >
> > Again, throught -> through.
>
> Fixed.
Again, the checked-in change is incorrect ("though").
(This file originally came from glibc, but glibc no longer uses this regex
implementation so there's no issue of fixing upstream ... really libiberty
ought to inherit from gnulib where possible, including for regex, but
that's a completely separate matter.)
--
Joseph S. Myers
jos...@codesourcery.com
Re: [Patch] Partially implement regex_search
On Mon, Jul 22, 2013 at 1:48 AM, Jonathan Wakely wrote: > OK. The rest of the patch is fine - would you like me to commit it for > you, with those fixes? I can commit on my own. I've already been a write-after-approval. Thank you again for reviewing! -- Tim Shen
Re: [Patch] Partially implement regex_search
-- Tim, while you work on these improvements, I think it would nice to also commit, when appropriate, testcases which came with bug reports which we closed as duplicates of the "std::regex is not implemented yet" report. I'm not referring to this specific commit, just a general comment. Thanks, Paolo.
[Patch] regex_iterator and regex_token_iterator implementation
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57513 is already in the testsuite and can passed(even before this patch?). To fully test it, a fully regex_search implementation is required. I'm working on a (damning) backtracking engine for it. -- Tim Shen iterators.patch Description: Binary data
Re: [Patch] regex_iterator and regex_token_iterator implementation
On 07/22/2013 02:47 AM, Tim Shen wrote: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57513 is already in the testsuite and can passed(even before this patch?). To fully test it, a fully regex_search implementation is required. I'm working on a (damning) backtracking engine for it. A couple of comments on the patch: the operator++, operator=, operator==, and _M_init are definitely too big to be inline. Also you are wrongly "un-uglyfying" many names, eg: - position_iterator __position; - const value_type* __result; - value_type__suffix; - std::size_t __n; - std::vector __subs; Remember all the names must begin by double underscore or single underscore, etc. Paolo.
[Patch, PR 57790] Waste work in can_move_insns_across()
Hi,
The problem appears in revision 201034 in version 4.9. I attached a
one-line patch that fixes it. I also reported this problem
at http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57790
I bootstrapped and ran the regression tests for this patch on x86_64-linux
and all tests pass.
In method "can_move_insns_across()" in df-problems.c, the loop on line
4038 should break immediately after "fail" is set to 1. All the iterations
after "fail" set to 1 do not perform any useful work, at best they just
set "fail" again to 1.
Index: gcc/df-problems.c
===
--- gcc/df-problems.c (revision 201034)
+++ gcc/df-problems.c (working copy)
@@ -4040,7 +4040,10 @@
if (i < FIRST_PSEUDO_REGISTER
&& ! fixed_regs[i]
&& ! global_regs[i])
- fail = 1;
+ {
+ fail = 1;
+ break;
+ }
}
}
-ChangIndex: gcc/df-problems.c
===
--- gcc/df-problems.c (revision 201034)
+++ gcc/df-problems.c (working copy)
@@ -4040,7 +4040,10 @@
if (i < FIRST_PSEUDO_REGISTER
&& ! fixed_regs[i]
&& ! global_regs[i])
- fail = 1;
+ {
+ fail = 1;
+ break;
+ }
}
}
Re: RFC: Gimple combine/folding interface
On Fri, Jul 19, 2013 at 5:09 PM, Andrew Pinski wrote:
> I was creating a new gimple/folding interface and wanted some opinions
> on the interface.
>
> typedef double_int (*nonzerobits_t)(tree var);
> typedef tree (*valueizer_t)(tree var);
>
> class gimple_combine
> {
> public:
> gimple_combine(nonzerobits_t a, valueizer_t b) : nonzerobitsf(a),
> valueizerv(b), allow_full_reassiocation(false) {}
> gimple_combine() : nonzerobitsf(NULL), valueizerv(NULL),
> allow_full_reassiocation(false) {}
> gimple_combine(bool reas) : nonzerobitsf(NULL), valueizerv(NULL),
> allow_full_reassiocation(reas) {}
> tree build2 (location_t, enum tree_code, tree, tree, tree);
> tree build1 (location_t, enum tree_code, tree, tree);
> tree build3 (location_t, enum tree_code, tree, tree, tree, tree);
> tree combine (gimple);
> private:
> nonzerobits_t nonzerobitsf;
> valueizer_t valueizerv;
> bool allow_full_reassiocation;
> tree binary (location_t, enum tree_code, tree, tree, tree);
> tree unary (location_t, enum tree_code, tree, tree);
> tree ternary (location_t, enum tree_code, tree, tree, tree, tree);
> };
>
> bool replace_rhs_after_ssa_combine (gimple_stmt_iterator *, tree);
>
> This is what I have so far and wonder if there is anything else I
> should include.
>
> This will be used to replace the folding code in fold-const.c and
> gimple-fold.c.
I placed what I decided in a branch in git:
http://gcc.gnu.org/git/?p=gcc.git;a=blob;f=gcc/gimple-ssa-combine.h;h=3d597291c1756cdf6e3318fd190ac70911ceb702;hb=d32468a31ab5e50fabab3a04303f6892ad890fd5
Note I won't have time to work on this again until the middle of August or so.
The status of the current patch is that it is able to build libgcc but
there are some addition ICEs due to checking to make sure all of the
forwprop optimizations are pushed over to gimple_combine (since I
started this work on an older code base).
Thanks,
Andrew Pinski
>
> Thanks,
> Andrew Pinski
Re: [ubsan] Add libcall arguments
On 07/21/2013 02:26 PM, Marek Polacek wrote: (pointer_sized_type_node): Define. Let's call it pointer_sized_int_node. Jason
Re: testsuite patches (1/14): Request dwarf-2 output where needed
On Sat, Jul 20, 2013 at 01:45:37AM -0400, Joern Rennecke wrote: > Although the gcc.dg/debug/dwarf2/dwarf2.exp generally requests dwarf-2 > information, so that the test will work with targets that have a different > default output format, that doesn't happen where the test specifies specific > target options. In that case, we have to specify -gdwarf-2 in the individual > test case. I ran into this problem a while ago, and after discussion on the mailing list, added a -gdwarf option to emit DWARF with the default version (http://gcc.gnu.org/ml/gcc-patches/2013-04/msg00592.html). Might be better than hardcoding dwarf-2 IMO. Regards Senthil
Re: testsuite patches (1/14): Request dwarf-2 output where needed
On Mon, Jul 22, 2013 at 10:31:03AM +0530, Senthil Kumar Selvaraj wrote: > On Sat, Jul 20, 2013 at 01:45:37AM -0400, Joern Rennecke wrote: > > Although the gcc.dg/debug/dwarf2/dwarf2.exp generally requests dwarf-2 > > information, so that the test will work with targets that have a different > > default output format, that doesn't happen where the test specifies specific > > target options. In that case, we have to specify -gdwarf-2 in the > > individual > > test case. > > I ran into this problem a while ago, and after discussion on the > mailing list, added a -gdwarf option to emit DWARF with the default > version (http://gcc.gnu.org/ml/gcc-patches/2013-04/msg00592.html). Might > be better than hardcoding dwarf-2 IMO. Yeah, definitely. Jakub
Re: [Patch] regex_iterator and regex_token_iterator implementation
On Mon, Jul 22, 2013 at 9:12 AM, Paolo Carlini wrote: > Also you are wrongly "un-uglyfying" many names, eg: > > - position_iterator __position; > - const value_type* __result; > - value_type__suffix; > - std::size_t __n; > - std::vector __subs; > > > Remember all the names must begin by double underscore or single underscore, > etc. Actually, in code before this patch, private members in class regex_iterator are indeed start without double or single underscore. It's because the standard (28.12, n3376) specifies them, though they are private. IMHO, they are part of standard, so I use names in it. Should I change them all to "_M_" or "__" format, and why? Thanks! -- Tim Shen
Re: [Patch] regex_iterator and regex_token_iterator implementation
Hi, >Should I change them all to "_M_" or "__" format, and why? Definitely. For the usual reason that if somebody in user code has a macro with the same name before including the header the code is busted. Of course _M_ or _S_ (vs _uppercase) is our specific convention for data members and statistics, but anyway *all* the names of implementation details must be uglified in a conforming implementation, no exceptions. Paolo
[ping] [PATCH] Fix pr57637
Ping? Is the updated patch OK for trunk? Thanks! -Zhenqiang On 16 July 2013 17:29, Zhenqiang Chen wrote: > On 11 July 2013 18:31, Eric Botcazou wrote: >>> Shrink-wrap optimization sinks some instructions for more >>> opportunities. It uses DF_LR_BB_INFO (bb)->def to check whether BB >>> clobbers SRC. But for ARM, gcc might generate cond_exec insns before >>> shrink-wrapping. And DF_LR_BB_INFO (bb)->def does not include def info >>> from cond_exec insns. So the check in function >>> move_insn_for_shrink_wrap is not enough. >> >> Posting a testcase would be even better, but the analysis looks plausible. >> DF_REF_CONDITIONAL and DF_REF_PARTIAL defs aren't killing defs so they are >> not >> included in the def set for LR. >> >> As you pointed out, they are included in the gen set for LIVE if it exists, >> so >> I guess we should wonder if we really want to do the scanning if LIVE doesn't >> exist, i.e. at -O1, instead of simply giving up. In any case, the scanning > > Thanks. > > The patch is updated based the comments: it will check GEN set of LIVE > if df_live exists. Otherwise, just give up. > > And a testcase is included in the attached patch. > > -Zhenqiang > > >> doesn't need to be redundant with the previous work, so: >> >> if (!(DF_REF_FLAGS (def) & (DF_REF_PARTIAL | DF_REF_CONDITIONAL))) >> continue; >> >> is missing in the patch. >> >> -- >> Eric Botcazou
Re: [Patch] regex_iterator and regex_token_iterator implementation
On Mon, Jul 22, 2013 at 1:29 PM, Paolo Carlini wrote: > Definitely. For the usual reason that if somebody in user code has a macro > with the same name before including the header the code is busted. Of course > _M_ or _S_ (vs _uppercase) is our specific convention for data members and > statistics, but anyway *all* the names of implementation details must be > uglified in a conforming implementation, no exceptions. > > Paolo > Great. This is the fixed version. -- Tim Shen iterators.patch Description: Binary data
Re: [Patch] regex_iterator and regex_token_iterator implementation
Hi, >Great. This is the fixed version. Patch is Ok with me but before committing please give a chance to Jon and other interested parties to have a look. Two more general comments: when you opencode more than a few lines don't hesitate to add comments too (remember that the new code must be maintained over the years and not necessarily by yourself ;) Also, at some point you should add appropriate .tcc headers for the out of line functions. Paolo
