Re: [4.7,trans-mem] Summary of unsolved known bugs
* Stack save/restore not working? -> XFAIL testcases Related to this: ICE: verify_gimple failed: invalid rhs for gimple memory store with -fgnu-tm --param tm-max-aggregate-size=32 http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51472 Proposed fix, waiting for review. [trans-mem] save/restore of thread-local data in nested txns is missing http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49581 Completely different than above. Will take a look. * [trans-mem] problem with TM clone aliases Leads to add _ITM_getTMCloneOrIrrevocable in a __transaction_atomic. -> Fix proposed. http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51516 Proposed fix, waiting for a review. * ICE: in function_and_variable_visibility, at ipa.c:835 with -O -fgnu-tm and overriding virtual transaction_safe function -> Fix proposed. http://gcc.gnu.org/ml/gcc-patches/2011-12/msg01182.html http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51411 Fixed. * ICE: verify_flow_info failed: BB 3 can not throw but has an EH edge with -fgnu-tm -fnon-call-exceptions and transaction_callable http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51212 Will take a look. * ICE when lto1 does not have -fgnu-tm and object file uses TM http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51280 I believe I wasn't able to reproduce this. * AIX unexpected error_mark node in new TM tests -> Probably already fixed? http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51174 fixed on trunk. Closed. Fixed but still open: * ICE: vector VEC(inline_summary_t,base) index domain error, in inline_summary at ipa-inline.h:193 with -O -fgnu-tm, transaction_safe and overloaded contructor http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51273 fixed, and closed * ICE: SIGSEGV in ix86_init_builtins (i386.c:27691) with -fgnu-tm and any fortran code http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51291 fixed and closed
Re: Modifying the datatype of a formal parameter
Hi, On Wed, Dec 21, 2011 at 12:53:10PM +1100, Matt Davis wrote: > Here is a follow up. I am closer to what I need, but not quite there > yet. Basically I just want to switch the type of one formal parameter > to a different type. > > On Mon, Dec 19, 2011 at 11:05 PM, Matt Davis wrote: > > Hi Martin and thank you very much for your reply. I do have some more > > resolution to my issue. > > > > On Mon, Dec 19, 2011 at 8:42 PM, Martin Jambor wrote: > >> Hi, > >> > >> On Sun, Dec 18, 2011 at 01:57:17PM +1100, Matt Davis wrote: > >>> I am using 'ipa_modify_formal_parameters()' to change the type of a > >>> function's > >>> formal parameter. After my pass completes, I get a 'gimple_expand_cfg()' > >>> error. I must be missing some key piece here, as the failure points to a > >>> NULL > >>> "SA.partition_to_pseudo" value. I also set_default_ssa_name() on the > >>> returned > >>> value from ipa_modify_formal_parameter (the adjustment's 'reduction' > >>> field). Do > >>> I need to re-gimplify the function or run some kind of 'cleanup' or > >>> 'update' > >>> once I modify this formal parameter? > >> > >> It's difficult to say without knowing what and at what stage of the > >> compilation you are doing. > > > > My pass is getting called as the last IPA pass > > (PLUGIN_ALL_IPA_PASSES_END). I do use the same function > > "ipa_modify_formal_parameters()" to add additional parameters to > > certain functions. And it works well. > > > >> The sad truth is that > >> ipa_modify_formal_parameters is very much crafted for its sole user > >> which is IPA-SRA and is probably quite less general than what the > >> original intention was. Any pass using the function then must modify > >> the body itself to reflect the changes, just like IPA-SRA does. > >> > >> SRA does not re-gimplify the modify functions, it just returns > >> TODO_update_ssa or (TODO_update_ssa | TODO_cleanup_cfg) if any EH > >> cleanup changed the CFG. > > > > Yep, and I do call update_ssa and cleanup_tree_cfg() after my pass. > > > >> So I would suggest to have a look at IPA-SRA (grep for the only call > >> to ipa_modify_formal_parameters in tree-sra.c), especially at what you > >> do differently. If you then have any further questions, feel free to > >> ask. > > > > Yeah, that was one of the first things I did. Now, as mentioned, I > > do have some more clarity on my issue. Basically, I am just changing > > the type of an existing formal parameter. When I look at > > "gimple_expand_cfg()" which is called later, I notice that the > > "SA.partition_to_pseudo" for that parameter is NULL, to which > > "gimple_expand_cfg()" aborts() on. Now, that value is NULL, because > > in "gimple_expand_cfg()" the function "expand_used_vars()" is called. > > I need "expand_one_var()" called since that should fix-up the RTX > > assigned to the parameter I am modifying. Unfortunately, the bitmap, > > "SA.partition_has_default_def" is true for the parameter, even if I do > > not set it explicitly. And since it is always set, the > > "expand_one_var()" routine is never called. I need to unset the > > default def associated to the param to force "expand_one_var()" to > > execute. So, for the ssa name assigned to the parameter I am > > modifying, I use SSA_NAME_IS_DEFAULT_DEF to set the flag to 'false' > > This sounds like a really gross hack. If I do this, I will need to > > set a new ssa definition for the modified parameter. > > I use ipa_modify_formal_paramaters() and swap the type of the param > with that of my desired type. The resulting PARM_DECL that the latter > function gives me has no default definition. So, I use > make_ssa_name() and set the return of that to the default definition > for the PARM_DECL. I am still not sure I understand but... if the uses of the old parameter could be quite arbitrary then you should not be inventing your own into-SSA mechanism. Just traverse all statements and replace all SSA_NAMEs of the old PARM_DECL with the new PARM_DECL (yes, with the DECL, don't bother creating any SSA_NAMES yourself). ipa_modify_formal_paramaters already calls mark_sym_for_renaming on the new parameter for you so you should be fine by just returning TODO_update_ssa from your pass. Is that not so? (Or perhaps replace only the default definition SSA_NAME, depending on what you actually want to do, but then you'd need to replace the others with a VAR_DECL, just like IPA-SRA does in replace_removed_params_ssa_names.) BTW, All this assumes that the types are register types (i.e. not aggregates like records or arrays), these never have SSA_NAMEs at all. Of course, nothing of the above dealt with that almost certainly there would be new type-casts necessary. But really you should not need to create your own SSA_NAMES for the new PARM_DECL, neither IPA-SRA nor IPA-CP/inlining do so. Martin
Re: Which Binutils should I use for performing daily regression test on trunk?
Terry Guo writes: > I plan to set up daily regression test on trunk for target > ARM-NONE-EABI and post results to gcc-testresults mailing list. Which > Binutils should I use, the Binutils trunk or the latest released > Binutils? And which way is recommended, building from a combined tree > or building separately? If there is something I should pay attention > to, please let me know. Thanks very much. For gcc testing, the latest released binutils is normally fine. You should only move to binutils trunk if there is some specific bug you need to work around temporarily. I personally would recommend building binutils separately. If you choose to build a combined tree, then you should ignore the previous paragraph and always use binutils trunk. For a combined tree you should always use sources from the same development date, so using gcc trunk implies using binutils trunk. Ian
Re: [4.7,trans-mem] Summary of unsolved known bugs
Wonderful! Thanks Aldy. On 12/21/2011 09:11 AM, Aldy Hernandez wrote: * ICE when lto1 does not have -fgnu-tm and object file uses TM http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51280 I believe I wasn't able to reproduce this. Arg really! For the openmp testcase, I was wrong but the tm testcase should show the problem. Please, can you (and maybe someone else) confirm that I am not the only one to see that? (Note I have started a thread here about that: http://gcc.gnu.org/ml/gcc-patches/2011-12/msg01221.html) Patrick.
Re: Possible wrong-way example in gcc4-4-2 documentation of __builtin_expect
On 21 December 2011 02:00, Jim Avera wrote:
> Ok, here is a patch which improves the example:
>
> --- gcc/doc/extend.texi.ORIG 2011-12-20 17:35:32.236578828 -0800
> +++ gcc/doc/extend.texi 2011-12-20 17:37:10.460583316 -0800
> @@ -7932,7 +7932,7 @@
>
> @smallexample
> if (__builtin_expect (ptr != NULL, 1))
> - error ();
> + ptr->do_something();
> @end smallexample
>
> @noindent
In order to follow the GCC coding style (a space between the function
name and opening parenthesis) and to match the first example for
__builtin_expect, I propose this patch instead:
Index: extend.texi
===
--- extend.texi (revision 182452)
+++ extend.texi (working copy)
@@ -7932,7 +7932,7 @@ expressions for @var{exp}, you should us
@smallexample
if (__builtin_expect (ptr != NULL, 1))
- error ();
+ ptr->foo ();
@end smallexample
@noindent
I've CC'd the gcc-patches list, which is where patches should be sent
for review, and included a ChangeLog entry:
2011-12-21 Jonathan Wakely
Jim Avera
* doc/extend.texi (__builtin_expect): Improve example.
Can I get approval to check this in to trunk?
>
>
>
> From: Jonathan Wakely
> To: Segher Boessenkool
> Cc: james_av...@yahoo.com; gcc@gcc.gnu.org
> Sent: Tuesday, December 20, 2011 5:22 AM
> Subject: Re: Possible wrong-way example in gcc4-4-2 documentation of
> __builtin_expect
>
> On 20 December 2011 12:49, Segher Boessenkool wrote:
>>
>> The point of the example is that you cannot write
>>
>> if (__builtin_expect (ptr, 1))
>> error ();
>>
>> so the "!= NULL" is important here. But you are right that
>> "error ()" is a bit unexpected; care to send a patch that changes
>> it to e.g. "do_something ()"?
>
> or even ptr->do_something() since that would depend on the value of ptr
Re: Possible wrong-way example in gcc4-4-2 documentation of __builtin_expect
On 21 December 2011 18:03, Jonathan Wakely wrote:
> On 21 December 2011 02:00, Jim Avera wrote:
>> Ok, here is a patch which improves the example:
>>
>> --- gcc/doc/extend.texi.ORIG 2011-12-20 17:35:32.236578828 -0800
>> +++ gcc/doc/extend.texi 2011-12-20 17:37:10.460583316 -0800
>> @@ -7932,7 +7932,7 @@
>>
>> @smallexample
>> if (__builtin_expect (ptr != NULL, 1))
>> - error ();
>> + ptr->do_something();
>> @end smallexample
>>
>> @noindent
>
> In order to follow the GCC coding style (a space between the function
> name and opening parenthesis) and to match the first example for
> __builtin_expect, I propose this patch instead:
>
> Index: extend.texi
> ===
> --- extend.texi (revision 182452)
> +++ extend.texi (working copy)
> @@ -7932,7 +7932,7 @@ expressions for @var{exp}, you should us
>
> @smallexample
> if (__builtin_expect (ptr != NULL, 1))
> - error ();
> + ptr->foo ();
> @end smallexample
>
> @noindent
Then again, maybe foo (*ptr) would be even better, so it looks more
like C not C++ code.
> I've CC'd the gcc-patches list, which is where patches should be sent
> for review, and included a ChangeLog entry:
>
> 2011-12-21 Jonathan Wakely
> Jim Avera
>
> * doc/extend.texi (__builtin_expect): Improve example.
>
>
> Can I get approval to check this in to trunk?
>
>
>
>>
>>
>>
>> From: Jonathan Wakely
>> To: Segher Boessenkool
>> Cc: james_av...@yahoo.com; gcc@gcc.gnu.org
>> Sent: Tuesday, December 20, 2011 5:22 AM
>> Subject: Re: Possible wrong-way example in gcc4-4-2 documentation of
>> __builtin_expect
>>
>> On 20 December 2011 12:49, Segher Boessenkool wrote:
>>>
>>> The point of the example is that you cannot write
>>>
>>> if (__builtin_expect (ptr, 1))
>>> error ();
>>>
>>> so the "!= NULL" is important here. But you are right that
>>> "error ()" is a bit unexpected; care to send a patch that changes
>>> it to e.g. "do_something ()"?
>>
>> or even ptr->do_something() since that would depend on the value of ptr
Re: Possible wrong-way example in gcc4-4-2 documentation of __builtin_expect
Jonathan Wakely writes:
> In order to follow the GCC coding style (a space between the function
> name and opening parenthesis) and to match the first example for
> __builtin_expect, I propose this patch instead:
>
> Index: extend.texi
> ===
> --- extend.texi (revision 182452)
> +++ extend.texi (working copy)
> @@ -7932,7 +7932,7 @@ expressions for @var{exp}, you should us
>
> @smallexample
> if (__builtin_expect (ptr != NULL, 1))
> - error ();
> + ptr->foo ();
> @end smallexample
>
> @noindent
>
>
> I've CC'd the gcc-patches list, which is where patches should be sent
> for review, and included a ChangeLog entry:
>
> 2011-12-21 Jonathan Wakely
> Jim Avera
>
> * doc/extend.texi (__builtin_expect): Improve example.
>
>
> Can I get approval to check this in to trunk?
This is fine, with or without your proposed change.
Thanks.
Ian
