Re: [testsuite] Adding target nonpic to g++.dg/tm/pr47746.C
*ping*
Alexander
2013/4/18 Patrick Marlier :
> Hi Alexander,
>
> On Thu, Apr 18, 2013 at 12:49 PM, Alexander Ivchenko
> wrote:
>> I'm trying it on linux/x86_64 on trunk. Testing just by adding -fpic
>> to the dg-options:
>>
>> --- a/gcc/testsuite/g++.dg/tm/pr47746.C
>> +++ b/gcc/testsuite/g++.dg/tm/pr47746.C
>> @@ -1,5 +1,5 @@
>> // { dg-do compile }
>> -// { dg-options "-fgnu-tm" }
>> +// { dg-options "-fgnu-tm -fpic" }
>>
>> Here is the error msg:
>>
>> testsuite/g++.dg/tm/pr47746.C:20:14: error: unsafe function call 'void
>> Building::load(InputStream*)' within 'transaction_safe' function
>
> Thanks! Now I understand the error (and I am able to reproduce it). :)
>
>> 4462| know what it will contain at runtime. */
>> 4463| if (cgraph_function_body_availability (node) < AVAIL_AVAILABLE)
>> 4464+>return true;
>> 4465|
>>
>> (gdb) p cgraph_function_body_availability (node)
>> $54 = AVAIL_OVERWRITABLE
>>
>> Sure I can file a PR If you think that the test was not supposed to
>> fail with -fpic
>
> I think your patch is OK but I cannot approve it since I am not a maintainer.
> I CCed Richard since he is the one who can approve and knows the TM
> implementation.
>
> Thanks,
> --
> Patrick
Re: [testsuite] Adding target nonpic to g++.dg/tm/pr47746.C
Oh, sorry. Missed the last msg from Mike in that thread.
2013/5/27 Alexander Ivchenko :
> *ping*
>
> Alexander
>
> 2013/4/18 Patrick Marlier :
>> Hi Alexander,
>>
>> On Thu, Apr 18, 2013 at 12:49 PM, Alexander Ivchenko
>> wrote:
>>> I'm trying it on linux/x86_64 on trunk. Testing just by adding -fpic
>>> to the dg-options:
>>>
>>> --- a/gcc/testsuite/g++.dg/tm/pr47746.C
>>> +++ b/gcc/testsuite/g++.dg/tm/pr47746.C
>>> @@ -1,5 +1,5 @@
>>> // { dg-do compile }
>>> -// { dg-options "-fgnu-tm" }
>>> +// { dg-options "-fgnu-tm -fpic" }
>>>
>>> Here is the error msg:
>>>
>>> testsuite/g++.dg/tm/pr47746.C:20:14: error: unsafe function call 'void
>>> Building::load(InputStream*)' within 'transaction_safe' function
>>
>> Thanks! Now I understand the error (and I am able to reproduce it). :)
>>
>>> 4462| know what it will contain at runtime. */
>>> 4463| if (cgraph_function_body_availability (node) < AVAIL_AVAILABLE)
>>> 4464+>return true;
>>> 4465|
>>>
>>> (gdb) p cgraph_function_body_availability (node)
>>> $54 = AVAIL_OVERWRITABLE
>>>
>>> Sure I can file a PR If you think that the test was not supposed to
>>> fail with -fpic
>>
>> I think your patch is OK but I cannot approve it since I am not a maintainer.
>> I CCed Richard since he is the one who can approve and knows the TM
>> implementation.
>>
>> Thanks,
>> --
>> Patrick
Re: RFA: fix rtl-optimization/56833
> Please find attached the updated patch. Thanks. Still, although reg_mode[regno] is indeed tested above, in + for (i = hard_regno_nregs[regno][mode] - 1; i > 0; i--) +if (reg_mode[i] != BLKmode) + return false; this should be reg_mode[regno + i] instead of reg_mode[i]. And one the nice benefits of the switch to C++ is that you can now declare the iteration variable within the 'for' construct. So, if you aren't planning to backport the patch to branches prior to 4.8, you can use the idiom in the newmove2add_record_mode and move2add_valid_value_p functions. The patch is OK with these modifications if it still passes testing. -- Eric Botcazou
Re: [PATCH] Do not allow non-top-level BIT_FIELD_REFs, IMAGPART_EXPRs or REALPART_EXPRs
On Fri, 24 May 2013, Martin Jambor wrote:
> Hi,
>
> On Thu, May 23, 2013 at 11:38:10AM +0200, Richard Biener wrote:
> > On Thu, 23 May 2013, Eric Botcazou wrote:
> >
> > > > earlier this week I asked on IRC whether we could have non-top-level
> > > > BIT_FIELD_REFs and Richi said that we could. However, when I later
> > > > looked at SRA code, quite apparently it is not designed to handle
> > > > non-top-level BIT_FIELD_REFs, IMAGPART_EXPRs or REALPART_EXPRs. So in
> > > > order to test whether that assumption is OK, I added the following
> > > > into the gimple verifier and ran bootstrap and testsuite of all
> > > > languages including Ada and ObjC++ on x86_64. It survived, which
> > > > makes me wondering whether we do not want it in trunk.
> > >
> > > This looks plausible to me, but I think that you ought to verify the real
> > > assumption instead, which is that the type of the 3 nodes is always
> > > scalar.
> > > The non-toplevelness of the nodes is merely a consequence of this
> > > property.
> >
> > Yeah. But please put the verification into tree-cfg.c:verify_expr
> > instead.
> >
>
> Like this? Also bootstrapped and tested on x86_64-linux.
>
> Thanks,
>
> Martin
>
>
> 2013-05-23 Martin Jambor
>
> * tree-cfg.c (verify_expr): Verify that BIT_FIELD_REFs, IMAGPART_EXPRs
> and REALPART_EXPRs have scalar type.
>
> Index: src/gcc/tree-cfg.c
> ===
> --- src.orig/gcc/tree-cfg.c
> +++ src/gcc/tree-cfg.c
> @@ -2669,10 +2669,17 @@ verify_expr (tree *tp, int *walk_subtree
>
> case REALPART_EXPR:
> case IMAGPART_EXPR:
> +case BIT_FIELD_REF:
> + if (!is_gimple_reg_type (TREE_TYPE (t)))
> + {
> + error ("non-scalar BIT_FIELD_REF, IMAGPART_EXPR or REALPART_EXPR");
> + return t;
> + }
> + /* Fall-through. */
> case COMPONENT_REF:
> case ARRAY_REF:
> case ARRAY_RANGE_REF:
> -case BIT_FIELD_REF:
> case VIEW_CONVERT_EXPR:
>/* We have a nest of references. Verify that each of the operands
>that determine where to reference is either a constant or a variable,
Yes, that looks good to me. Note that this still does not verify
that REALPART_EXPR, IMAGPART_EXPR and BIT_FIELD_REF are only
outermost handled-component refs. It merely verifies that if they
are outermost then they are not aggregate.
Thus a followup would be to move the BIT_FIELD_REF handling in the
loop below to the above case sub-set and disallow BIT_FIELD_REF,
REALPART_EXPR and IMAGPART_EXPR inside that loop.
Though I'm pretty sure that evetually this will fail ...
The patch is ok, it's an improvement over the current state.
Thanks,
Richard.
Re: PR tree-optimization/57337
On Sun, May 26, 2013 at 5:53 AM, Easwaran Raman wrote:
> On Sat, May 25, 2013 at 4:46 AM, Richard Biener
> wrote:
>> Easwaran Raman wrote:
>>
>>>In that case, if my insert_stmt immediately follows dep_stmt and both
>>>have the same UID, not_dominated_by would return true and I will end
>>>up updating insert_stmt to dep_stmt which is wrong.
>>
>> But there should be a safe default answer for
>> Equal uids. Unless we are asking different questions in different places.
>> Thus, I do not like the stmt walking but rather have a safe fallback.
>
> I am lost here. I don't see how we could avoid doing the stmt walking
> to resolve the equal uid case. How to ensure that not_dominated_by (a,
> b) returns true and not_dominated_by (b, a) returns false if A and B
> have the same UID and A appears before B without doing the statement
> walk. And, I don't see why the statement walk is bad. It is not likely
> that there is a long sequence of statements with the same UID.
Sure, but if you are always asking a question like "is placing X before Y ok?"
then you can conservatively answer "no" and code should handle that ok.
If you are asking questions both way then of course no conservative answer is
possible. Both current uses of not_dominated_by are of the same kind,
if the placement is not ok then either the insert point needs adjustment or
the debug stmt reset.
Richard.
> Thanks,
> Easwaran
>
>>
>> Richard.
>>
>>>- Easwaran
>>>
>>>On Fri, May 24, 2013 at 1:07 AM, Richard Biener
>>> wrote:
On Thu, May 23, 2013 at 7:26 PM, Easwaran Raman
>>>wrote:
> This addresses the case where UID alone is not sufficient to figure
> out which statement appears earlier in a BB. Bootstraps and no test
> regressions in x86_64 on linux. Ok for trunk?
Why not simply conservatively use gimple_uid (a) <= gimple_uid (b)
in not_dominated_by?
Richard.
> Thanks,
> Easwaran
>
>
> 2013-05-23 Easwaran Raman
>
> PR tree-optimization/57337
> * tree-ssa-reassoc.c (appears_later_in_bb): New function.
> (find_insert_point): Correctly identify the insertion point
> when two statements with the same UID is compared.
>
> Index: gcc/tree-ssa-reassoc.c
> ===
> --- gcc/tree-ssa-reassoc.c (revision 199211)
> +++ gcc/tree-ssa-reassoc.c (working copy)
> @@ -2866,6 +2866,31 @@ not_dominated_by (gimple a, gimple b)
>
> }
>
> +/* Among STMT1 and STMT2, return the statement that appears later.
>>>Both
> + statements are in same BB and have the same UID. */
> +
> +static gimple
> +appears_later_in_bb (gimple stmt1, gimple stmt2)
> +{
> + unsigned uid = gimple_uid (stmt1);
> + gimple_stmt_iterator gsi = gsi_for_stmt (stmt1);
> + gsi_next (&gsi);
> + if (gsi_end_p (gsi))
> +return stmt1;
> + for (; !gsi_end_p (gsi); gsi_next (&gsi))
> +{
> + gimple stmt = gsi_stmt (gsi);
> +
> + /* If STMT has a different UID than STMT1 and we haven't seen
> + STMT2 during traversal, we know STMT1 appears later. */
> + if (gimple_uid (stmt) != uid)
> +return stmt1;
> + else if (stmt == stmt2)
> +return stmt2;
> +}
> + gcc_unreachable ();
> +}
> +
> /* Find the statement after which STMT must be moved so that the
> dependency from DEP_STMT to STMT is maintained. */
>
> @@ -2875,7 +2900,11 @@ find_insert_point (gimple stmt, gimple
>>>dep_stmt)
>gimple insert_stmt = stmt;
>if (dep_stmt == NULL)
> return stmt;
> - if (not_dominated_by (insert_stmt, dep_stmt))
> + if (gimple_uid (insert_stmt) == gimple_uid (dep_stmt)
> + && gimple_bb (insert_stmt) == gimple_bb (dep_stmt)
> + && insert_stmt != dep_stmt)
> +insert_stmt = appears_later_in_bb (insert_stmt, dep_stmt);
> + else if (not_dominated_by (insert_stmt, dep_stmt))
> insert_stmt = dep_stmt;
>return insert_stmt;
> }
>>
>>
Re: Fix PR 53743 and other -freorder-blocks-and-partition failures
A nitpick: the dragon book was first published 36 years ago... (!) -miles -- “There are moments, Jeeves, when one asks oneself, ‘Do trousers matter?’” “The mood will pass, sir.” [P.G. Wodehouse, "The Code Of The Woosters"]
Re: [PATCH] Enable non-complex math builtins from C99 for Bionic
Hi, While discussing the issue with the command line option for sincos here: http://gcc.1065356.n5.nabble.com/PATCH-bionic-Add-foptimize-sincos-tp940918.html Richard wrote: > I'd rather think about a way to specify, for all known builtins, whether GCC > should generate calls to such function where they are not in the source > program. That is, similar to how we have -f[no-]builtin-FOO introduce > -f[no-]libc-FOO (with a better name for 'libc'?). That way there would be > a way to specify that -floop-distribute-patterns should not produce calls > to memset () for example. This patch is related to that idea: in the target hook libc_has_function we can check whether the compiler should generate calls to a particular function or not. The command line support for each function could be the next step. Could you please take a look? thanks Alexander 2013/4/23 Alexander Ivchenko : > *ping* > > thanks > Alexander > > 2013/3/28 Alexander Ivchenko : >> Hi, >> >> 4.8 is now branched, lets come back to the discussion that we had >> before. I updated the patch a little >> bit since we now have linux-protos.h and linux-android.c files. >> >> I tried to preserve the avaiability of c99 for all targets, but it's >> pretty difficult, because we are changing >> the defaults. Passing an empty string as second argument doesn't look >> very good, but on the other hand >> the user has one clear way for checking the presence of a certain >> function. But of course we can create >> another function, that will call targetm.libc_has_function >> (function_class, "") within itself. >> >> best regards, >> Alexander >> >> 2013/1/7 Joseph S. Myers : >>> On Fri, 21 Dec 2012, Alexander Ivchenko wrote: >>> Hi, Thank you very much for your input! Please, take a look at the updated version: I fixed coding style, moved documentation for TARGET_LIBC_HAS_FUNCTION to target.def. Removed TARGET_C99_FUNCTIONS and TARGET_HAS_SINCOS and all their influence and moved the implementation of linux_libc_has_function to host-linux.c. I changed the defaults: now it is assumed that we have C99 runtime, but no sincos. I updated all needed gcc/config/*.h. But 'm not sure in this part, cause I don't have the opportunity to test it properly... >>> >>> This patch seems mostly plausible, though there are various places that >>> call targetm.libc_has_function with and empty string as second argument, >>> that should be naming the specific function instead. I haven't reviewed >>> the details, and at this development stage I think it will need to wait >>> until after 4.8 branches. >>> >>> -- >>> Joseph S. Myers >>> jos...@codesourcery.com
[libgomp] PR57423 - correct argument names in omp_lib.f90 and .texi
The dummy arguments names in a Fortran interface block are part of the
interface as one can use "call proc(argname=value)" besides "call
proc(value)"; thus, the dummy argument names are significant.
The attached patch updates the names in the module file - and for
completeness also in the documentation.
OK for the trunk?
Tobias
PS: That the *_lock arguments are called "lock" in C/C++ but "svar" and
"nvar" in Fortran is an odd inconsistency in the OpenMP spec.
2013-05-27 Tobias Burnus
PR fortran/57423
* libgomp.texi (omp_set_dynamic, omp_set_nested, omp_set_nested,
omp_set_num_threads, omp_init_lock, omp_set_lock, omp_test_lock,
omp_unset_lock, omp_destroy_lock, omp_init_nest_lock,
omp_set_nest_lock, omp_test_nest_lock, omp_unset_nest_lock,
omp_destroy_nest_lock): Correct arguments to match the one in
the OpenMP spec.
* omp_lib.f90.in (omp_init_lock, omp_init_nest_lock, omp_destroy_lock
omp_destroy_nest_lock, omp_set_lock, omp_set_nest_lock, omp_unset_lock,
omp_unset_nest_lock, omp_set_dynamic, omp_set_nested,
omp_set_num_threads, omp_test_lock, omp_test_nest_lock): Ditto.
diff --git a/libgomp/libgomp.texi b/libgomp/libgomp.texi
index 40c3830..2985128 100644
--- a/libgomp/libgomp.texi
+++ b/libgomp/libgomp.texi
@@ -601,13 +601,13 @@ adjustment of team sizes and @code{false} disables it.
@item @emph{C/C++}:
@multitable @columnfractions .20 .80
-@item @emph{Prototype}: @tab @code{void omp_set_dynamic(int set);}
+@item @emph{Prototype}: @tab @code{void omp_set_dynamic(int dynamic_threads);}
@end multitable
@item @emph{Fortran}:
@multitable @columnfractions .20 .80
-@item @emph{Interface}: @tab @code{subroutine omp_set_dynamic(set)}
-@item @tab @code{logical, intent(in) :: set}
+@item @emph{Interface}: @tab @code{subroutine omp_set_dynamic(dynamic_threads)}
+@item @tab @code{logical, intent(in) :: dynamic_threads}
@end multitable
@item @emph{See also}:
@@ -657,13 +657,13 @@ dynamic adjustment of team sizes and @code{false} disables it.
@item @emph{C/C++}:
@multitable @columnfractions .20 .80
-@item @emph{Prototype}: @tab @code{void omp_set_nested(int set);}
+@item @emph{Prototype}: @tab @code{void omp_set_nested(int nested);}
@end multitable
@item @emph{Fortran}:
@multitable @columnfractions .20 .80
-@item @emph{Interface}: @tab @code{subroutine omp_set_nested(set)}
-@item @tab @code{logical, intent(in) :: set}
+@item @emph{Interface}: @tab @code{subroutine omp_set_nested(nested)}
+@item @tab @code{logical, intent(in) :: nested}
@end multitable
@item @emph{See also}:
@@ -685,13 +685,13 @@ argument of @code{omp_set_num_threads} shall be a positive integer.
@item @emph{C/C++}:
@multitable @columnfractions .20 .80
-@item @emph{Prototype}: @tab @code{void omp_set_num_threads(int n);}
+@item @emph{Prototype}: @tab @code{void omp_set_num_threads(int num_threads);}
@end multitable
@item @emph{Fortran}:
@multitable @columnfractions .20 .80
-@item @emph{Interface}: @tab @code{subroutine omp_set_num_threads(n)}
-@item @tab @code{integer, intent(in) :: n}
+@item @emph{Interface}: @tab @code{subroutine omp_set_num_threads(num_threads)}
+@item @tab @code{integer, intent(in) :: num_threads}
@end multitable
@item @emph{See also}:
@@ -750,8 +750,8 @@ an unlocked state.
@item @emph{Fortran}:
@multitable @columnfractions .20 .80
-@item @emph{Interface}: @tab @code{subroutine omp_init_lock(lock)}
-@item @tab @code{integer(omp_lock_kind), intent(out) :: lock}
+@item @emph{Interface}: @tab @code{subroutine omp_init_lock(svar)}
+@item @tab @code{integer(omp_lock_kind), intent(out) :: svar}
@end multitable
@item @emph{See also}:
@@ -779,8 +779,8 @@ a deadlock occurs.
@item @emph{Fortran}:
@multitable @columnfractions .20 .80
-@item @emph{Interface}: @tab @code{subroutine omp_set_lock(lock)}
-@item @tab @code{integer(omp_lock_kind), intent(inout) :: lock}
+@item @emph{Interface}: @tab @code{subroutine omp_set_lock(svar)}
+@item @tab @code{integer(omp_lock_kind), intent(inout) :: svar}
@end multitable
@item @emph{See also}:
@@ -809,8 +809,8 @@ does not block if the lock is not available. This function returns
@item @emph{Fortran}:
@multitable @columnfractions .20 .80
-@item @emph{Interface}: @tab @code{logical function omp_test_lock(lock)}
-@item @tab @code{integer(omp_lock_kind), intent(inout) :: lock}
+@item @emph{Interface}: @tab @code{logical function omp_test_lock(svar)}
+@item @tab @code{integer(omp_lock_kind), intent(inout) :: svar}
@end multitable
@item @emph{See also}:
@@ -839,8 +839,8 @@ again, set the lock to itself.
@item @emph{Fortran}:
@multitable @columnfractions .20 .80
-@item @emph{Interface}: @tab @code{subroutine omp_unset_lock(lock)}
-@item @tab @code{integer(omp_lock_kind), intent(inout) :: loc
Re: [libgomp] PR57423 - correct argument names in omp_lib.f90 and .texi
On Mon, May 27, 2013 at 11:21:37AM +0200, Tobias Burnus wrote: > The dummy arguments names in a Fortran interface block are part of > the interface as one can use "call proc(argname=value)" besides > "call proc(value)"; thus, the dummy argument names are significant. > > The attached patch updates the names in the module file - and for > completeness also in the documentation. > > OK for the trunk? > > Tobias > > PS: That the *_lock arguments are called "lock" in C/C++ but "svar" > and "nvar" in Fortran is an odd inconsistency in the OpenMP spec. Ok, thanks. > 2013-05-27 Tobias Burnus > > PR fortran/57423 > * libgomp.texi (omp_set_dynamic, omp_set_nested, omp_set_nested, > omp_set_num_threads, omp_init_lock, omp_set_lock, omp_test_lock, > omp_unset_lock, omp_destroy_lock, omp_init_nest_lock, > omp_set_nest_lock, omp_test_nest_lock, omp_unset_nest_lock, > omp_destroy_nest_lock): Correct arguments to match the one in > the OpenMP spec. > * omp_lib.f90.in (omp_init_lock, omp_init_nest_lock, omp_destroy_lock > omp_destroy_nest_lock, omp_set_lock, omp_set_nest_lock, omp_unset_lock, > omp_unset_nest_lock, omp_set_dynamic, omp_set_nested, > omp_set_num_threads, omp_test_lock, omp_test_nest_lock): Ditto. Jakub
[ping] couple of pending patches
Hi, Add workaround for LEON 3 FP errata (for the builtins.c bits): http://gcc.gnu.org/ml/gcc-patches/2012-11/msg01950.html Introduce #pragma GCC diagnostic off (for the diagnostic.c bits): http://gcc.gnu.org/ml/gcc-patches/2013-04/msg01149.html Thanks in advance. -- Eric Botcazou
Re: [Patch, Fortran] PR57217 - re-add type checks for TBP overriding
Sorry for taking so long to come back to this (I was traveling all of last week) ... >> Ok, so: How about the attached patch as a simple & backportable fix for >> the regression? (Ok for trunk/4.8/4.7?) > > I think that part is okay - but as you mentioned TYPE(*) in your last email: > That doesn't work; I think compare_type_rank should be made asymmetrical in > this regard (ditto for "!gcc$ attributes no_arg_check"). Thus, could you fix > that part as well? What do you mean? That "type(t)" arguments should be able to override "type(*)", but not vice versa? I think that would interfere with the current patch, which symmetrizes the call to "compare_type_rank". If one direction gives a negative result, then both cases will be rejected. Anyway, anything in this direction is probably a non-regression and should rather be handled as a follow-up. Is the current patch (http://gcc.gnu.org/ml/fortran/2013-05/msg00045.html) ok for trunk/4.8/4.7? Cheers, Janus
[PATCH] Fix PR57343
The following fixes PR57343 - niter analysis uses multiple_of_p to
check whether a value is a multiple of another, but multiple_of_p
does not refrain from returning true for a * 10 and 10 even when
the multiplication may overflow (and changing that would pessimize
some callers at least while possibly fixing a latent bug in
extract_mul_div). So the following guards the use of
multiple_of_p in niter analysis properly and implements the
important special-cases for wrapping types.
Bootstrapped and tested on x86_64-unknown-linux-gnu, ok for trunk?
Thanks,
Richard.
2013-05-27 Richard Biener
PR tree-optimization/57343
* tree-ssa-loop-niter.c (number_of_iterations_ne_max): Do not
use multiple_of_p if not TYPE_OVERFLOW_UNDEFINED.
(number_of_iterations_cond): Do not build the folded tree.
* gcc.dg/torture/pr57343.c: New testcase.
Index: gcc/tree-ssa-loop-niter.c
===
*** gcc/tree-ssa-loop-niter.c (revision 199350)
--- gcc/tree-ssa-loop-niter.c (working copy)
*** number_of_iterations_ne_max (mpz_t bnd,
*** 552,561
{
double_int max;
mpz_t d;
bool bnds_u_valid = ((no_overflow && exit_must_be_taken)
|| mpz_sgn (bnds->below) >= 0);
! if (multiple_of_p (TREE_TYPE (c), c, s))
{
/* If C is an exact multiple of S, then its value will be reached before
the induction variable overflows (unless the loop is exited in some
--- 552,569
{
double_int max;
mpz_t d;
+ tree type = TREE_TYPE (c);
bool bnds_u_valid = ((no_overflow && exit_must_be_taken)
|| mpz_sgn (bnds->below) >= 0);
! if (integer_onep (s)
! || (TREE_CODE (c) == INTEGER_CST
! && TREE_CODE (s) == INTEGER_CST
! && tree_to_double_int (c).mod (tree_to_double_int (s),
!TYPE_UNSIGNED (type),
!EXACT_DIV_EXPR).is_zero ())
! || (TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (c))
! && multiple_of_p (type, c, s)))
{
/* If C is an exact multiple of S, then its value will be reached before
the induction variable overflows (unless the loop is exited in some
*** number_of_iterations_ne_max (mpz_t bnd,
*** 572,587
the whole # of iterations analysis will fail). */
if (!no_overflow)
{
! max = double_int::mask (TYPE_PRECISION (TREE_TYPE (c))
!- tree_low_cst (num_ending_zeros (s), 1));
mpz_set_double_int (bnd, max, true);
return;
}
/* Now we know that the induction variable does not overflow, so the loop
iterates at most (range of type / S) times. */
! mpz_set_double_int (bnd, double_int::mask (TYPE_PRECISION (TREE_TYPE (c))),
! true);
/* If the induction variable is guaranteed to reach the value of C before
overflow, ... */
--- 580,594
the whole # of iterations analysis will fail). */
if (!no_overflow)
{
! max = double_int::mask (TYPE_PRECISION (type)
! - tree_low_cst (num_ending_zeros (s), 1));
mpz_set_double_int (bnd, max, true);
return;
}
/* Now we know that the induction variable does not overflow, so the loop
iterates at most (range of type / S) times. */
! mpz_set_double_int (bnd, double_int::mask (TYPE_PRECISION (type)), true);
/* If the induction variable is guaranteed to reach the value of C before
overflow, ... */
*** number_of_iterations_cond (struct loop *
*** 1311,1317
}
/* If the loop exits immediately, there is nothing to do. */
! if (integer_zerop (fold_build2 (code, boolean_type_node, iv0->base,
iv1->base)))
{
niter->niter = build_int_cst (unsigned_type_for (type), 0);
niter->max = double_int_zero;
--- 1318,1325
}
/* If the loop exits immediately, there is nothing to do. */
! tree tem = fold_binary (code, boolean_type_node, iv0->base, iv1->base);
! if (tem && integer_zerop (tem))
{
niter->niter = build_int_cst (unsigned_type_for (type), 0);
niter->max = double_int_zero;
Index: gcc/testsuite/gcc.dg/torture/pr57343.c
===
*** gcc/testsuite/gcc.dg/torture/pr57343.c (revision 0)
--- gcc/testsuite/gcc.dg/torture/pr57343.c (working copy)
***
*** 0
--- 1,22
+ /* { dg-do run } */
+
+ int c = 0;
+
+ int
+ main ()
+ {
+ int i, f = 1;
+ for (i = 0; i < 5; i++)
+ {
+ --c;
+ unsigned char h = c * 100;
+ if (h == 0)
+ {
+ f = 0;
+ break;
+ }
+ }
+ if (f != 1)
+ __builtin_abort ();
+ return 0;
+ }
[PATCH] Fix PR57417, alternate fix for PR57381
Independent on the outcome of the discussion regarding to comparing
ADDR_EXPRs in operand_equal_p the following presents a more
backportable fix for the SCCVN endless loop. We know that we
compare is_gimple_min_invariant addresses which means we can
resort to a get_addr_base_and_unit_offset comparison to double-check
the operand_equal_p result.
For now I'll revert the previous change to the operand_equal_p behavior.
Bootstrap / regtest pending on x86_64-unknown-linux-gnu.
Richard.
2013-05-27 Richard Biener
Revert
PR middle-end/57381
* fold-const.c (operand_equal_p): Compare FIELD_DECLs with
OEP_CONSTANT_ADDRESS_OF retained.
PR tree-optimization/57417
* tree-ssa-sccvn.c (vn_reference_fold_indirect): Fix test
for unchanged base.
(set_ssa_val_to): Compare addresses using
get_addr_base_and_unit_offset.
* gcc.dg/torture/pr57417.c: New testcase.
Index: gcc/fold-const.c
===
*** gcc/fold-const.c(revision 199350)
--- gcc/fold-const.c(working copy)
*** operand_equal_p (const_tree arg0, const_
*** 2664,2673
case COMPONENT_REF:
/* Handle operand 2 the same as for ARRAY_REF. Operand 0
may be NULL when we're called to compare MEM_EXPRs. */
! if (!OP_SAME_WITH_NULL (0) || !OP_SAME (1))
return 0;
flags &= ~OEP_CONSTANT_ADDRESS_OF;
! return OP_SAME_WITH_NULL (2);
case BIT_FIELD_REF:
if (!OP_SAME (0))
--- 2664,2673
case COMPONENT_REF:
/* Handle operand 2 the same as for ARRAY_REF. Operand 0
may be NULL when we're called to compare MEM_EXPRs. */
! if (!OP_SAME_WITH_NULL (0))
return 0;
flags &= ~OEP_CONSTANT_ADDRESS_OF;
! return OP_SAME (1) && OP_SAME_WITH_NULL (2);
case BIT_FIELD_REF:
if (!OP_SAME (0))
Index: gcc/tree-ssa-sccvn.c
===
*** gcc/tree-ssa-sccvn.c(revision 199350)
--- gcc/tree-ssa-sccvn.c(working copy)
*** vn_reference_fold_indirect (vecop0, 0),
&addr_offset);
gcc_checking_assert (addr_base && TREE_CODE (addr_base) != MEM_REF);
! if (addr_base != op->op0)
{
double_int off = tree_to_double_int (mem_op->op0);
off = off.sext (TYPE_PRECISION (TREE_TYPE (mem_op->op0)));
--- 1145,1151
addr_base = get_addr_base_and_unit_offset (TREE_OPERAND (op->op0, 0),
&addr_offset);
gcc_checking_assert (addr_base && TREE_CODE (addr_base) != MEM_REF);
! if (addr_base != TREE_OPERAND (op->op0, 0))
{
double_int off = tree_to_double_int (mem_op->op0);
off = off.sext (TYPE_PRECISION (TREE_TYPE (mem_op->op0)));
*** static inline bool
*** 2608,2613
--- 2608,2614
set_ssa_val_to (tree from, tree to)
{
tree currval = SSA_VAL (from);
+ HOST_WIDE_INT toff, coff;
if (from != to)
{
*** set_ssa_val_to (tree from, tree to)
*** 2643,2649
print_generic_expr (dump_file, to, 0);
}
! if (currval != to && !operand_equal_p (currval, to, OEP_PURE_SAME))
{
VN_INFO (from)->valnum = to;
if (dump_file && (dump_flags & TDF_DETAILS))
--- 2644,2660
print_generic_expr (dump_file, to, 0);
}
! if (currval != to
! && !operand_equal_p (currval, to, 0)
! /* ??? For addresses involving volatile objects or types
operand_equal_p
! does not reliably detect ADDR_EXPRs as equal. We know we are only
!getting invariant gimple addresses here, so can use
!get_addr_base_and_unit_offset to do this comparison. */
! && !(TREE_CODE (currval) == ADDR_EXPR
! && TREE_CODE (to) == ADDR_EXPR
! && (get_addr_base_and_unit_offset (TREE_OPERAND (currval, 0), &coff)
! == get_addr_base_and_unit_offset (TREE_OPERAND (to, 0), &toff))
! && coff == toff))
{
VN_INFO (from)->valnum = to;
if (dump_file && (dump_flags & TDF_DETAILS))
Index: gcc/testsuite/gcc.dg/torture/pr57417.c
===
*** gcc/testsuite/gcc.dg/torture/pr57417.c (revision 0)
--- gcc/testsuite/gcc.dg/torture/pr57417.c (working copy)
***
*** 0
--- 1,12
+ /* { dg-do compile } */
+
+ int a, b;
+ volatile int *c;
+
+ void foo ()
+ {
+ volatile int d[1];
+ b = 0;
+ for (;; a--)
+ c = &d[b];
+ }
Re: [ping] couple of pending patches
On Mon, May 27, 2013 at 11:52 AM, Eric Botcazou wrote: > Hi, > > Add workaround for LEON 3 FP errata (for the builtins.c bits): > http://gcc.gnu.org/ml/gcc-patches/2012-11/msg01950.html Ok. Thanks, Richard. > Introduce #pragma GCC diagnostic off (for the diagnostic.c bits): > http://gcc.gnu.org/ml/gcc-patches/2013-04/msg01149.html > > Thanks in advance. > > -- > Eric Botcazou
Re: [Patch, Fortran] PR57217 - re-add type checks for TBP overriding
Janus Weil wrote: Ok, so: How about the attached patch as a simple & backportable fix for the regression? (Ok for trunk/4.8/4.7?) I think that part is okay - but as you mentioned TYPE(*) in your last email: That doesn't work; I think compare_type_rank should be made asymmetrical in this regard (ditto for "!gcc$ attributes no_arg_check"). Thus, could you fix that part as well? What do you mean? Try the four attached test cases - they should all be rejected, but they are accepted. Anyway, anything in this direction is probably a non-regression and should rather be handled as a follow-up. Is the current patch (http://gcc.gnu.org/ml/fortran/2013-05/msg00045.html) ok for trunk/4.8/4.7? OK. Still, I would like if the attached test cases would be rejected. (The first one only affects GCC 4.9, the others also GCC4.8). Tobias PS: If you have time, could you please review http://gcc.gnu.org/ml/fortran/2013-05/msg00081.html (dealloc intent(in) poly array)? (For the finalization wrapper patch, I will later send a follow-up patch which addresses some additional issues, I found while testing.) module base_mod type base_type integer :: kind contains procedure, pass(map) :: clone=> base_clone end type base_type contains subroutine base_clone(map,mapout,info) implicit none class(base_type), intent(inout) :: map class(base_type), intent(inout) :: mapout integer :: info end subroutine base_clone end module base_mod module r_mod use base_mod type, extends(base_type) :: r_type real :: dat contains procedure, pass(map) :: clone=> r_clone end type r_type contains subroutine r_clone(map,mapout,info) implicit none class(r_type), intent(inout) :: map !gcc$ attributes no_arg_check :: mapout integer, intent(inout) :: mapout integer :: info end subroutine r_clone end module r_mod module base_mod type base_type integer :: kind contains procedure, pass(map) :: clone=> base_clone end type base_type contains subroutine base_clone(map,mapout,info) implicit none class(base_type), intent(inout) :: map class(base_type), intent(inout) :: mapout integer :: info end subroutine base_clone end module base_mod module r_mod use base_mod type, extends(base_type) :: r_type real :: dat contains procedure, pass(map) :: clone=> r_clone end type r_type contains subroutine r_clone(map,mapout,info) implicit none class(r_type), intent(inout) :: map class(base_type), intent(inout) :: mapout(..) integer :: info end subroutine r_clone end module r_mod module base_mod type base_type integer :: kind contains procedure, pass(map) :: clone=> base_clone end type base_type contains subroutine base_clone(map,mapout,info) implicit none class(base_type), intent(inout) :: map class(base_type), intent(inout) :: mapout integer :: info end subroutine base_clone end module base_mod module r_mod use base_mod type, extends(base_type) :: r_type real :: dat contains procedure, pass(map) :: clone=> r_clone end type r_type contains subroutine r_clone(map,mapout,info) implicit none class(r_type), intent(inout) :: map type(*), intent(inout) :: mapout integer :: info end subroutine r_clone end module r_mod
[PING] [PATCH RX] Added target specific macros for RX100, RX200, and RX600
Hi, Below patch added target specific macros for RX100, RX200, and RX600 http://gcc.gnu.org/ml/gcc-patches/2013-05/msg00050.html Please review the patch and let me know if there should be any modifications in it? Regards, Sandeep Kumar Singh, KPIT Cummins InfoSystems Ltd. Pune, India
[Patch, Fortran] Mark intrinsics called by the finalization wrapper as attr.artificial
Testing showed that the finalization wrapper caused errors of the kind:
Error: The intrinsic '_F.rank' declared INTRINSIC at (1) is not
available in the current standard settings but new in TS 29113. Use an
appropriate -std=* option or enable -fall-intrinsics in order to use it.
That's fixed by the attached patch, which I have committed as obvious
(Rev. 199355). (I have a test case, which depends on a not yet submitted
patch. The artificial cannot interfere with user functions as GFC_PREFIX
gives a symtree which cannot interfere with a normal Fortran name.)
Tobias
Index: gcc/fortran/ChangeLog
===
--- gcc/fortran/ChangeLog (Revision 199354)
+++ gcc/fortran/ChangeLog (Arbeitskopie)
@@ -1,3 +1,8 @@
+2013-05-27 Tobias Burnus
+
+ * expr.c (gfc_build_intrinsic_call): Make symbol as attr.artificial.
+ * intrinsic.c (gfc_is_intrinsic): Disable std check for those.
+
2013-05-22 Tobias Burnus
* resolve.c (get_temp_from_expr): Change mangling to
Index: gcc/fortran/expr.c
===
--- gcc/fortran/expr.c (Revision 199354)
+++ gcc/fortran/expr.c (Arbeitskopie)
@@ -4657,6 +4657,7 @@ gfc_build_intrinsic_call (gfc_namespace *ns, gfc_i
result->symtree->n.sym->intmod_sym_id = id;
result->symtree->n.sym->attr.flavor = FL_PROCEDURE;
result->symtree->n.sym->attr.intrinsic = 1;
+ result->symtree->n.sym->attr.artificial = 1;
va_start (ap, numarg);
atail = NULL;
Index: gcc/fortran/intrinsic.c
===
--- gcc/fortran/intrinsic.c (Revision 199354)
+++ gcc/fortran/intrinsic.c (Arbeitskopie)
@@ -1046,7 +1046,8 @@ gfc_is_intrinsic (gfc_symbol* sym, int subroutine_
return false;
/* See if this intrinsic is allowed in the current standard. */
- if (!gfc_check_intrinsic_standard (isym, &symstd, false, loc))
+ if (!gfc_check_intrinsic_standard (isym, &symstd, false, loc)
+ && !sym->attr.artificial)
{
if (sym->attr.proc == PROC_UNKNOWN
&& gfc_option.warn_intrinsics_std)
Re: [PATCH] Fix PR57343
Hi, > Bootstrapped and tested on x86_64-unknown-linux-gnu, ok for trunk? > > PR tree-optimization/57343 > * tree-ssa-loop-niter.c (number_of_iterations_ne_max): Do not > use multiple_of_p if not TYPE_OVERFLOW_UNDEFINED. > (number_of_iterations_cond): Do not build the folded tree. > > * gcc.dg/torture/pr57343.c: New testcase. ok, Zdenek
Re: [testsuite] Adding target nonpic to g++.dg/tm/pr47746.C
Hi, This is just a memo about why the testcase failed: When it tries to compile, we get this: gcc/testsuite/g++.dg/tm/pr47746.C:20:14: error: unsafe function call ‘void Building::load(InputStream*)’ within ‘transaction_safe’ function load(stream); Indeed, with PIC, the 'load' method can be overloaded later so we cannot infer that the body will be transaction_safe. in trans-mem.c: 4461| /* If we aren't seeing the final version of the function we don't 4462| know what it will contain at runtime. */ 4463| if (cgraph_function_body_availability (node) < AVAIL_AVAILABLE) 4464+>return true; The two ways to fix this, is either to force for non-pic target or to add transaction_safe attribute to 'load' and 'readUint32' methods. -- Patrick
[PATCH] Fix PR57412, OMP loop creation
This fixes the latch block used for the new loop in
expand_omp_atomic_pipeline.
Bootstrap / regtest pending on x86_64-unknown-linux-gnu.
Richard.
2013-05-27 Richard Biener
PR middle-end/57412
* omp-low.c (expand_omp_atomic_pipeline): Use the correct latch
block for the new loop.
* gcc.dg/gomp/pr57412.c: New testcase.
Index: gcc/omp-low.c
===
*** gcc/omp-low.c (revision 199356)
--- gcc/omp-low.c (working copy)
*** expand_omp_atomic_pipeline (basic_block
*** 5665,5671
struct loop *loop = alloc_loop ();
loop->header = loop_header;
! loop->latch = loop_header;
add_loop (loop, loop_header->loop_father);
if (gimple_in_ssa_p (cfun))
--- 5665,5671
struct loop *loop = alloc_loop ();
loop->header = loop_header;
! loop->latch = store_bb;
add_loop (loop, loop_header->loop_father);
if (gimple_in_ssa_p (cfun))
Index: gcc/testsuite/gcc.dg/gomp/pr57412.c
===
*** gcc/testsuite/gcc.dg/gomp/pr57412.c (revision 0)
--- gcc/testsuite/gcc.dg/gomp/pr57412.c (working copy)
***
*** 0
--- 1,10
+ /* { dg-do compile } */
+
+ int thr;
+ #pragma omp threadprivate (thr)
+ int foo ()
+ {
+ int l;
+ #pragma omp parallel copyin (thr) reduction (||:l)
+ ;
+ }
Re: [PATCH 0/2] Proof-of-concept towards removal of the "cfun" global
On Sat, May 25, 2013 at 9:33 PM, Jan Hubicka wrote:
>> Here's an idea that could make it easier to remove the "cfun" global.
>>
>> "cfun" is a major piece of global state within gcc: it's the 5th most
>> accessed variable in the build (accessed in ~4600 places within one stage
>> of a build, based on [1]). This is an obstacle to making gcc's code be
>> usable as a library.
>
> Yep, note that cfun is not the only beast of this. For 99% of backend it
> needs
> to be kept in sync with current_function_decl pointing to the declaration such
> that DECL_STRUCT_FUNCTION (current_function_decl) = cfun.
> There is also implicit notion of "current cgraph node" that is accessed by
> cgraph_get_node (current_function_decl) many times that can get expensive
> since it leads to hashtable lookup.
> Finally there is "crtl" that is macro hidding a global variable x_rtl
> data for RTL data of the currently compiled function.
>
> In many ways these four are all closely related and data in between them
> are not distributed in very engineered way.
Note that ideally there exists a single global variable per compilation phase.
Frontends would use current_function_decl (as struct function is not available
very early) while the middle-end and target would use cfun.
Through cfun you can reach its decl and you should ideally be able to reach
the cgraph node, too.
>> I can think of three approaches to "cfun":
>> (a) status quo: a global variable, with macros to prevent direct
>> assignment, and an API for changing cfun.
>
> We have push_cfun/pop_cfun for that. If you did not look into what they
> really do, you may be disappointed. They are huge&expensive beasts switching
> a lot of global state in compiler for optimize attribute. So it means that
> on the track removing them we will need to cleanup this area, too.
Yeah ...
>> (b) have a global "context" or "universe" object, and put cfun in
>> there (perhaps with tricks to be able to make this a singleton in a
>> non-library build, optimizing away the context lookups somehow
>> - see [2] for discussion on this)
>> (c) go through all of the places where cfun is used, and somehow ensure
>> that they're passed in the data they need. Often it's not the
>> function that's used, but its cfg.
>>
>> I think (c) is the ideal from a modularity perspective (it unlocks the
>> ability to have optimization passes be working on different functions in
>> different threads), but the most difficult.
>
> (c) is direction I was trying to push IPA code to in longer run. We need
> a single object holding "function" and that should be passed to most
> of the macros.
>
> I am not entirely happy with contents of the "struct function" at all
> (it is kind of kitchen sink for all the data we randomly need to associate
> with function), but it is closest to it. Most of IPA node considers the
> ultimate function pointer to be the cgraph node however.
>
> We do not want to melt those all thogether, since the data in there have
> different
> lifetime. In particular struct_function is not in memory during WPA.
I agree to all of the above.
It would be interesting to have a static analyzer compute what are the most
executed functions that ultimately end up referencing cfun ...
>>
>> One part of the puzzle is that various header files in the build define
>> macros that reference the "cfun" global, e.g.:
>>
>> #define n_basic_blocks (cfun->cfg->x_n_basic_blocks)
>>
>> This one isn't in block caps, which might mislead a new contributor into
>> thinking it's a variable, rather than a macro, so there may be virtue in
>
> Yep yo umany notice there are already _FN variants for that. The macors
> are lower case since historically they were variables. I agree they should
> be replaced.
Yes, explicit "cfun" usage is way better than implicit one - especially because
we still have implicit uses in functions that are already being passed the
function context as parameter ...
>> removing these macros for that reason alone. (I know that these confused
>> me for a while when I first started writing my plugin) [3]
>>
>> So I had a go at removing these macros, to make usage of "cfun" be
>> explicit.
>>
>> I wrote a script to do most of the gruntwork automatically: [4]
>>
>> The following patches introduce accessor methods to control_flow_graph,
>> then remove all of the macros that reference cfun from basic-block.h,
>> replacing all of the places that use them with explicit uses of
>> "cfun->cfg->get_foo ()" as appropriate. There are various other headers
>> that define macros that use cfun, but I thought I'd post this to get a
>> sense of how maintainers feel about this approach.
>>
>> I notice that we're constantly accessing:
>>
>> some loop
>> {
>> ...
>> use cfun->cfg->x_some_field;
>> ...
>> }
>>
>> Would it potentially be faster to replace some of these with:
>>
>> control_flow_graph &cfg = *cfun->cfg;
>> some loop
>> {
>> ...
>
Re: [PATCH 0/2] Proof-of-concept towards removal of the "cfun" global
> > I think that was one reason crtl was introduced ... to avoid an indirection > through cfun. Main reason why I introduced crtl was to get struct function less bloated. RTL data live for only one function body at a time. It is useless to store them in a datastructure that is allocated at parsing time and freed at very end of compilation. I never did really serious analysis on performance implications of having crtl a static viarable or pointer or pointer from cfun. With current threading efforts, I guess we can either make RTL optimizers to have their own RTL context (i.e. crtl pointer passed everywhere) or just have crtl pointer in cfun (and get extra indirection + waste 8 bytes on every function body). > > I indeed welcome the effort to make all 'cfun' uses explicit. Even more > welcome > is reducing the number of 'cfun' references ;) Like giving all gimple/rtl > pass > execute () / gate () functions a struct function argument (or even > better abstract > that so we can add more context transparently later, like dump_file / > dump_flags). Yep. One of issues is that for IPA/RTL/Gimple passes the notion of context is always slightly different and not exactly matching the purpose of a function in the IL/symbol table. Honza
Re: [PATCH 0/2] Proof-of-concept towards removal of the "cfun" global
On Mon, May 27, 2013 at 3:41 PM, Jan Hubicka wrote: >> >> I think that was one reason crtl was introduced ... to avoid an indirection >> through cfun. > > Main reason why I introduced crtl was to get struct function less bloated. > RTL data live for only one function body at a time. It is useless to store > them > in a datastructure that is allocated at parsing time and freed at very end of > compilation. > > I never did really serious analysis on performance implications of having crtl > a static viarable or pointer or pointer from cfun. With current threading > efforts, I guess we can either make RTL optimizers to have their own RTL > context (i.e. crtl pointer passed everywhere) or just have crtl pointer in > cfun > (and get extra indirection + waste 8 bytes on every function body). >> >> I indeed welcome the effort to make all 'cfun' uses explicit. Even more >> welcome >> is reducing the number of 'cfun' references ;) Like giving all gimple/rtl >> pass >> execute () / gate () functions a struct function argument (or even >> better abstract >> that so we can add more context transparently later, like dump_file / >> dump_flags). > > Yep. One of issues is that for IPA/RTL/Gimple passes the notion of context is > always slightly different and not exactly matching the purpose of a function > in the IL/symbol table. Yeah, well. The idea still holds that it would be nice to annotate passes that are "safe" (do not reference cfun in any way) and NULL cfun from the pass manager for that passes. Of course IPA passes still access the global symtab_nodes array - a bit to go until one compiler instance can process multiple TUs at once ;) Richard. > Honza
Re: [PATCH 1/2] handwritten part of patch
On Sat, May 25, 2013 at 3:02 PM, David Malcolm wrote:
> Eliminate all direct references to "cfun" from macros in
> basic-block.h and introduce access methods to control_flow_graph
>
> * basic-block.h (control_flow_graph::get_basic_block_by_idx): New
> accessor methods.
> (control_flow_graph::get_entry_block): New method.
> (control_flow_graph::get_exit_block): New method.
> (control_flow_graph::get_basic_block_info): New methods.
> (control_flow_graph::get_n_basic_blocks): New methods.
> (control_flow_graph::get_n_edges): New methods.
> (control_flow_graph::get_last_basic_block): New methods.
> (control_flow_graph::get_label_to_block_map): New methods.
> (control_flow_graph::get_profile_status): New method.
> (control_flow_graph::set_profile_status): New method.
> (ENTRY_BLOCK_PTR): Eliminate this macro.
> (EXIT_BLOCK_PTR): Likewise.
> (basic_block_info): Likewise.
> (n_basic_blocks): Likewise.
> (n_edges): Likewise.
> (last_basic_block): Likewise.
> (label_to_block_map): Likewise.
> (profile_status): Likewise.
> (BASIC_BLOCK): Likewise.
> (SET_BASIC_BLOCK): Likewise.
> (FOR_EACH_BB_FN): Rewrite in terms of...
> (FOR_EACH_BB_CFG): New macro
> (FOR_EACH_BB): Eliminate this macro
> (FOR_EACH_BB_REVERSE_FN): Rewrite in terms of...
> (FOR_EACH_BB_REVERSE_FN_CFG): New macro
> (FOR_EACH_BB_REVERSE): Eliminate this macro
> (FOR_ALL_BB): Likewise.
> (FOR_ALL_BB_CFG): New macro
I don't like the mix of _CFG / _FN. It's obvious we are talking about
the CFG of a function in 'FOR_EACH_BB' and friends.
> ---
> diff --git a/gcc/basic-block.h b/gcc/basic-block.h
> index eed320c..3949417 100644
> --- a/gcc/basic-block.h
> +++ b/gcc/basic-block.h
> @@ -276,6 +276,57 @@ enum profile_status_d
> fields of this struct are interpreted as the defines for backward
> source compatibility following the definition of this struct. */
> struct GTY(()) control_flow_graph {
> +public:
> + basic_block get_basic_block_by_idx (int idx) const
> + {
> +return (*x_basic_block_info)[idx];
> + }
get_basic_block_by_idx is rather long, any reason to not shorten
it to get_block () or get_bb?
> + void set_basic_block_by_idx (int idx, basic_block bb)
set_block () or set_bb()
> + {
> +(*x_basic_block_info)[idx] = bb;
> + }
> +
> + basic_block get_entry_block () const { return x_entry_block_ptr; }
> +
> + basic_block get_exit_block () const { return x_exit_block_ptr; }
> +
> + vec *get_basic_block_info () const
are they really used outside of the iterators?
> + {
> +return x_basic_block_info;
> + }
> + vec *&get_basic_block_info ()
> + {
> +return x_basic_block_info;
> + }
> +
> + int get_n_basic_blocks () const { return x_n_basic_blocks; }
> + int& get_n_basic_blocks () { return x_n_basic_blocks; }
> +
> + int get_n_edges () const { return x_n_edges; }
> + int& get_n_edges () { return x_n_edges; }
> +
> + int get_last_basic_block () const { return x_last_basic_block; }
> + int& get_last_basic_block () { return x_last_basic_block; }
As you can't set_ those I'd drop the get_. Any reason to not simply
drop the x_ in the members and allow direct accesses? I fear
an -O0 compiler will get even more un-debuggable that it is now
with all the accessors :/ (undebuggable as in stepping through source)
> + vec *get_label_to_block_map () const
> + {
> +return x_label_to_block_map;
> + }
> + vec *&get_label_to_block_map ()
> + {
> +return x_label_to_block_map;
> + }
> +
> + enum profile_status_d get_profile_status () const
> + {
> +return x_profile_status;
> + }
> + void set_profile_status (enum profile_status_d status)
> + {
> +x_profile_status = status;
> + }
> +
> +public:
>/* Block pointers for the exit and entry of a function.
> These are always the head and tail of the basic block list. */
>basic_block x_entry_block_ptr;
> @@ -328,32 +379,20 @@ struct GTY(()) control_flow_graph {
> #define SET_BASIC_BLOCK_FOR_FUNCTION(FN,N,BB) \
>((*basic_block_info_for_function(FN))[(N)] = (BB))
>
> -/* Defines for textual backward source compatibility. */
> -#define ENTRY_BLOCK_PTR(cfun->cfg->x_entry_block_ptr)
> -#define EXIT_BLOCK_PTR (cfun->cfg->x_exit_block_ptr)
> -#define basic_block_info (cfun->cfg->x_basic_block_info)
> -#define n_basic_blocks (cfun->cfg->x_n_basic_blocks)
> -#define n_edges(cfun->cfg->x_n_edges)
> -#define last_basic_block (cfun->cfg->x_last_basic_block)
> -#define label_to_block_map (cfun->cfg->x_label_to_block_map)
> -#define profile_status (cfun->cfg->x_profile_status)
> -
> -#define BASIC_BLOCK(N) ((*basic_block_info)[(N)])
> -#define SET_BASIC_BLOCK(N,BB) ((*basic_block_info)[(N)] = (BB))
> -
Replacements
Re: [patch,fortran] PR50405 - Statement function with itself as argument SEGV's
Am 26.05.2013 05:58, schrieb Bud Davis:
The changes suggested by:
http://gcc.gnu.org/ml/fortran/2013-05/msg00057.html
have been made in the below diff and test file.
Could someone please commit this ?
I thought you have commit rights?
Note: You added a hyphen to the error message without changing the
dg-error pattern. Additionally, it would help if you could also create a
changelog for the testsuite - also a "*" was missing before the file
name in the ChangeLog. And finally, I am not sure whether "!{ dg-do
compile }" works - it might be that DejaGNU requires a space before the "{".
Committed as Rev. 199358 (with minor modifications). Thanks for the
patch - and welcome back to contributing to gfortran :-)
Tobias
Index: gcc/fortran/ChangeLog
===
--- gcc/fortran/ChangeLog (Revision 199357)
+++ gcc/fortran/ChangeLog (Arbeitskopie)
@@ -1,3 +1,9 @@
+2013-05-27 Bud Davis
+
+ PR fortran/50405
+ * resolve.c (resolve_formal_arglist): Detect error when an argument
+ has the same name as the function.
+
2013-05-27 Tobias Burnus
* expr.c (gfc_build_intrinsic_call): Make symbol as attr.artificial.
Index: gcc/fortran/resolve.c
===
--- gcc/fortran/resolve.c (Revision 199357)
+++ gcc/fortran/resolve.c (Arbeitskopie)
@@ -306,6 +306,14 @@ resolve_formal_arglist (gfc_symbol *proc)
&& !resolve_procedure_interface (sym))
return;
+ if (strcmp (proc->name, sym->name) == 0)
+{
+ gfc_error ("Self-referential argument "
+ "'%s' at %L is not allowed", sym->name,
+ &proc->declared_at);
+ return;
+}
+
if (sym->attr.if_source != IFSRC_UNKNOWN)
resolve_formal_arglist (sym);
Index: gcc/testsuite/ChangeLog
===
--- gcc/testsuite/ChangeLog (Revision 199357)
+++ gcc/testsuite/ChangeLog (Arbeitskopie)
@@ -1,3 +1,8 @@
+2013-05-27 Bud Davis
+
+ PR fortran/50405
+ * gfortran.dg/stfunc_8.f90: New.
+
2013-05-27 Richard Biener
PR tree-optimization/57343
Index: gcc/testsuite/gfortran.dg/stfunc_8.f90
===
--- gcc/testsuite/gfortran.dg/stfunc_8.f90 (Revision 0)
+++ gcc/testsuite/gfortran.dg/stfunc_8.f90 (Arbeitskopie)
@@ -0,0 +1,9 @@
+! { dg-do compile }
+! { dg-options "" }
+!
+! PR fortran/50405
+!
+! Submitted by zec...@gmail.com
+!
+ f(f) = 0 ! { dg-error "Self-referential argument" }
+ end
Re: [DWARF] Fix multiple register spanning location.
On 05/16/2013 12:27 AM, Cary Coutant wrote:
>>> How about using dbx_reg_number (XVECEXP (regs, 0, i)) instead? The
>>> bare use of DBX_REGISTER_NUMBER earlier in that function is protected
>>> by a gcc_assert, but this one isn't.
>>
For the respective targets maintainers that drop into the thread: Here
is a summary of the problem:
Since http://gcc.gnu.org/ml/gcc-patches/2012-03/msg00209.html, dwarf
double floating point registers are not correctly described for the SH.
But this patch was needed to fix an assertion in the dwarf2cfi.
Therefore, we have a discrepancy between the different targets, that can
result in assertions, (or possibly silent wrong unwind code I believe)
SH,MIPS,C6X ; dwarf_register_span returns hard_reg numbers. regno is
never translated for DBX
ARM NEON ; converts regno into DBX
numbers
POWERPC; ? returns boths.
So a second set of patches
http://gcc.gnu.org/ml/gcc-patches/2013-05/msg01230.html
http://gcc.gnu.org/ml/gcc-patches/2013-05/msg00312.html
fixed it with a common rule. All interfaces are changed to return
hard_reg numbers only. multiple_reg_location is in charge of calling
DBX_REGISTER_NUMBER with an assertion check.
Well, in fact this was not doing some good for the powerpc, that now
asserts here. The problem is that rs6000_dwarf_register_span stores in
the PARALLEL rtx both the hard reg and the dbx reg, so we can't call
dbx_reg_number in it.
Using DBX_REGISTER_NUMBER instead of dbx_reg_number restores the
previous working status for all targets. This is
dwarf-span-assert-rs6000.patch for reference.
However I feel a little bit uncomfortable with this solution that
doesn't seem to fix the root cause. The dbx_register_number hooks is
called basically from two places : dwarf2cfi.c and dwarf2out.c. That
show different uses: either we want to refer to the hard regno when
dealing with the cfa description (whereas we want DWARF_FRAME_REGNUM,
not DBX_REGISTER_NUMBER). or we use the DBX_REGISTER_NUMBER for output
register locations.
Since this information cannot be detected contextually, I'd like to
extend the dwarf_register_span target hook to return a dbx number or
not. This is dwarf-span-target-dbx.patch
build tested with the configurations that failed at one time or the other:
- sh64-unknown-elf (The original sh64-elf build failure assertion in
dwarf2cfi is fixed.)
- arm-none-eabi -with-fpu=neon-vfpv4
- powerpc-e500v2-linux-gnuspe
- x86_64-unknown-linux-gnu sanity build OK
Is dwarf-span-target-dbx.patch OK for trunk ?. More comments ?
Many Thanks,
Christian
2013-05-23 Christian Bruel
PR debug/57351
PR debug/57389
* config/arm/arm.c (arm_dwarf_register_span): Add bool dbx parameter.
* config/c6x/c6x.c (c6x_dwarf_register_span): Likewise.
* config/mips/mips (mips_dwarf_register_span): Likewise.
* config/rs6000/rs6000.c (rs6000_dwarf_register_span): Likewise.
* config/sh/sh.c (sh_dwarf_register_span): Likewise. Declare static.
* config/sh/sh-protos.h (sh_dwarf_register_span): Remove declaration.
* gcc/doc/tm.texi: Add bool dbx parameter.
* gcc/target.def: Likewise,
* gcc/dwarf2cfi.c (dwarf2out_frame_debug_cfa_offset): Don't span dbx.
(dwarf2out_frame_debug_cfa_expression): Don't span dbx.
* gcc/dwarf2out.c (reg_loc_descriptor): Span dbx.
* gcc/hooks.c: (hook_rtx_bool_null): Define.
* gcc/hooks.h: (hook_rtx_bool_null): Declare.
Index: gcc/config/arm/arm.c
===
--- gcc/config/arm/arm.c (revision 199354)
+++ gcc/config/arm/arm.c (working copy)
@@ -213,7 +213,7 @@ static bool arm_output_ttype (rtx);
static void arm_asm_emit_except_personality (rtx);
static void arm_asm_init_sections (void);
#endif
-static rtx arm_dwarf_register_span (rtx);
+static rtx arm_dwarf_register_span (rtx, bool);
static tree arm_cxx_guard_type (void);
static bool arm_cxx_guard_mask_bit (void);
@@ -25855,7 +25855,7 @@ arm_dbx_register_number (unsigned int regno)
GCC models tham as 64 32-bit registers, so we need to describe this to
the DWARF generation code. Other registers can use the default. */
static rtx
-arm_dwarf_register_span (rtx rtl)
+arm_dwarf_register_span (rtx rtl, bool dbx)
{
unsigned regno;
int nregs;
@@ -25878,6 +25878,8 @@ static rtx
nregs = GET_MODE_SIZE (GET_MODE (rtl)) / 8;
p = gen_rtx_PARALLEL (VOIDmode, rtvec_alloc (nregs));
+ if (dbx)
+regno = 256 + (regno - FIRST_VFP_REGNUM) / 2;
for (i = 0; i < nregs; i++)
XVECEXP (p, 0, i) = gen_rtx_REG (DImode, regno + i);
Index: gcc/config/c6x/c6x.c
===
--- gcc/config/c6x/c6x.c (revision 199354)
+++ gcc/config/c6x/c6x.c (working copy)
@@ -6304,7 +6304,7 @@ c6x_set_return_address (rtx source, rtx scratch)
registers for DWARF generation code. */
static rtx
-c6x_dwarf_register_span (rtx rtl)
+c6x_dwarf_register_span (rtx rtl, bool dbx ATTRIBUTE
Re: [Patch, Fortran] Enable the generation of the FINALization wrapper function
Small re-diff - but essentially unchanged.
(I made a thinko when adding a _final call to
gfc_trans_class_array_init_assign: Not in all contexts the _final should
be called, only for INTENT(OUT). Thus, I remove the _final call and
deferred it to the actual finalization call. [That also matches the
scalar handling, which only does a memcpy and no dealloc.])
Build and regtested on x86-64-gnu-linux.
OK for the trunk?
Tobias
Tobias Burnus wrote:
Pre-remark: This patch does *not* enable finalization or polymorphic
deallocation.
* * *
Dear all,
The attached patch is a bit boring and invasive, but it paves the way
to FINAL support.
Changes of technical kind:
* Changed ABI for CLASS's virtual table (due to _final) - and, hence,
it bumps the .mod version
* The finalization wrapper is now generated (this should not but might
lead to ICEs)
* It also causes that the virtual table is now more often generated
New feature:
_copy no longer deallocates the "dst" argument. Doing so lead to bogus
finalization with ALLOCATE (exposed with the pending FINAL patch). As
a sideeffect, memset could be removed and CALLOC could be replased by
MALLOC (minute performance advantage). In order to keep the
deallocation in gfc_trans_class_array_init_assign, there is now a call
to the finalization wrapper.
Next steps:
* Add end-of-scope/intent(out) deallocation for polymorphic arrays
* Enable FINAL parsing
* Stepwise enabling for polymorphic deallocation/finalization
* Fix issues with ELEMENTAL(+optional) with intent(out)
* Fix some issues related to intrinsic assignment
* Fix fallout of any of those items
Build and regtested on x86-64-gnu-linux.
OK for the trunk?
Tobias
2013-05-27 Tobias Burnus
* class.c (finalize_component): Fix coarray array refs.
(gfc_find_intrinsic_vtab): _copy's dst is now intent(inout).
(gfc_find_derived_vtab): Ditto. Enable finalization-wrapper
generation.
* module.c (MOD_VERSION): Bump.
(gfc_dump_module, gfc_use_module): Remove empty line in .mod.
* trans-array.c (gfc_conv_descriptor_token): Accept nonrestricted
void pointer.
(gfc_array_allocate, structure_alloc_comps): Don't nullify for
BT_CLASS allocations.
* trans-stmt.c (gfc_trans_allocate): Ditto.
2013-05-27 Tobias Burnus
* gfortran.dg/auto_dealloc_2.f90: Update _free count in the dump.
* gfortran.dg/class_19.f03: Ditto.
diff --git a/gcc/fortran/class.c b/gcc/fortran/class.c
index 349f494..c41b95a 100644
--- a/gcc/fortran/class.c
+++ b/gcc/fortran/class.c
@@ -832,17 +832,18 @@ finalize_component (gfc_expr *expr, gfc_symbol *derived, gfc_component *comp,
ref->u.c.component = comp;
e->ts = comp->ts;
- if (comp->attr.dimension
+ if (comp->attr.dimension || comp->attr.codimension
|| (comp->ts.type == BT_CLASS && CLASS_DATA (comp)
- && CLASS_DATA (comp)->attr.dimension))
+ && (CLASS_DATA (comp)->attr.dimension
+ || CLASS_DATA (comp)->attr.codimension)))
{
ref->next = gfc_get_ref ();
ref->next->type = REF_ARRAY;
- ref->next->u.ar.type = AR_FULL;
ref->next->u.ar.dimen = 0;
ref->next->u.ar.as = comp->ts.type == BT_CLASS ? CLASS_DATA (comp)->as
: comp->as;
e->rank = ref->next->u.ar.as->rank;
+ ref->next->u.ar.type = e->rank ? AR_FULL : AR_ELEMENT;
}
/* Call DEALLOCATE (comp, stat=ignore). */
@@ -2363,7 +2364,7 @@ gfc_find_derived_vtab (gfc_symbol *derived)
dst->attr.flavor = FL_VARIABLE;
dst->attr.dummy = 1;
dst->attr.artificial = 1;
- dst->attr.intent = INTENT_OUT;
+ dst->attr.intent = INTENT_INOUT;
gfc_set_sym_referenced (dst);
copy->formal->next = gfc_get_formal_arglist ();
copy->formal->next->sym = dst;
@@ -2382,9 +2383,6 @@ gfc_find_derived_vtab (gfc_symbol *derived)
components and the calls to finalization subroutines.
Note: The actual wrapper function can only be generated
at resolution time. */
- /* FIXME: Enable ABI-breaking "_final" generation. */
- if (0)
- {
if (!gfc_add_component (vtype, "_final", &c))
goto cleanup;
c->attr.proc_pointer = 1;
@@ -2392,7 +2390,6 @@ gfc_find_derived_vtab (gfc_symbol *derived)
c->tb = XCNEW (gfc_typebound_proc);
c->tb->ppc = 1;
generate_finalization_wrapper (derived, ns, tname, c);
- }
/* Add procedure pointers for type-bound procedures. */
if (!derived->attr.unlimited_polymorphic)
@@ -2651,7 +2648,7 @@ gfc_find_intrinsic_vtab (gfc_typespec *ts)
dst->ts.kind = ts->kind;
dst->attr.flavor = FL_VARIABLE;
dst->attr.dummy = 1;
- dst->attr.intent = INTENT_OUT;
+ dst->attr.intent = INTENT_INOUT;
gfc_set_sym_referenced (dst);
copy->formal->next = gfc_get_formal_arglist ();
copy->formal->next->sym = dst;
diff --git a/gcc/fortran/module.c b/gcc/fortran/module.c
index e6a4cd7..9486b28 100644
--- a/gcc/fortran/module.c
+++ b/gcc/fortran/module.c
@@ -80,10 +80,8 @@ along with GCC; see the file COPYING3. I
[Patch, Fortran] Enable FINAL parsing support (and nothing more)
Dear all,
the attached patch now enables the *parsing* of "FINAL ::" - however,
*no* finalization will be done. Adding FINALIZATION will be deferred to
several follow-up patches.
Build on x86-64-gnu-linux.
OK for the trunk?
* * *
Patches pending to be reviewed, which are also required for finalization:
a) Deallocate intent(out),allocatable polymorphic arrays,
http://gcc.gnu.org/ml/fortran/2013-05/msg00081.html
b) Enable the generation of the finalization wrapper,
http://gcc.gnu.org/ml/fortran/2013-05/msg00093.html
(I created the patches in the order (a) -> (b) -> this patch. However,
each patch should be independent of another.)
I prefer to have them in the trunk to expose the (b) some real-life
testing and to make it simpler to continue with the next step:
After the three patches have been committed, the complete finalization
infrastructure is finally available and enabled. Minus bugs ;-), "only"
the calls to the finalizer wrapper has to be added at various spots,
e.g. for intent(out) (allocatable and not, honoring element+optional),
end of scope (allocatable and not), explicit deallocate, intrinsic
assignment (w/ and w/o realloc on assign, with
allocatable/nonallocatable/coarray components, etc. - and all items for
polymorphic and nonpolymorphic variables).
Tobias
2013-05-27 Tobias Burnus
PR fortran/37336
* resolve.c (gfc_resolve_finalizers): Remove not implemented error.
2013-05-27 Tobias Burnus
PR fortran/37336
* gfortran.dg/finalize_11.f90: New.
* gfortran.dg/finalize_4.f03: Remove dg-error.
* gfortran.dg/finalize_5.f03: Ditto.
* gfortran.dg/finalize_6.f03: Ditto.
* gfortran.dg/finalize_7.f03: Ditto.
diff --git a/gcc/fortran/resolve.c b/gcc/fortran/resolve.c
index 78a1038..4cfc858 100644
--- a/gcc/fortran/resolve.c
+++ b/gcc/fortran/resolve.c
@@ -11241,10 +11241,6 @@ error:
" defined at %L, suggest also scalar one",
derived->name, &derived->declared_at);
- /* TODO: Remove this error when finalization is finished. */
- gfc_error ("Finalization at %L is not yet implemented",
- &derived->declared_at);
-
gfc_find_derived_vtab (derived);
return result;
}
diff --git a/gcc/testsuite/gfortran.dg/finalize_4.f03 b/gcc/testsuite/gfortran.dg/finalize_4.f03
index 11e094f..b4c08f2 100644
--- a/gcc/testsuite/gfortran.dg/finalize_4.f03
+++ b/gcc/testsuite/gfortran.dg/finalize_4.f03
@@ -48,6 +48,3 @@ PROGRAM finalizer
DEALLOCATE(mat)
END PROGRAM finalizer
-
-! TODO: Remove this once finalization is implemented.
-! { dg-excess-errors "not yet implemented" }
diff --git a/gcc/testsuite/gfortran.dg/finalize_5.f03 b/gcc/testsuite/gfortran.dg/finalize_5.f03
index b9ec376..fb81531 100644
--- a/gcc/testsuite/gfortran.dg/finalize_5.f03
+++ b/gcc/testsuite/gfortran.dg/finalize_5.f03
@@ -107,6 +107,3 @@ PROGRAM finalizer
IMPLICIT NONE
! Nothing here, errors above
END PROGRAM finalizer
-
-! TODO: Remove this once finalization is implemented.
-! { dg-excess-errors "not yet implemented" }
diff --git a/gcc/testsuite/gfortran.dg/finalize_6.f90 b/gcc/testsuite/gfortran.dg/finalize_6.f90
index 82d662f..d155c7b 100644
--- a/gcc/testsuite/gfortran.dg/finalize_6.f90
+++ b/gcc/testsuite/gfortran.dg/finalize_6.f90
@@ -10,9 +10,9 @@ MODULE final_type
TYPE :: mytype
INTEGER :: fooarr(42)
REAL :: foobar
- CONTAINS ! { dg-error "Fortran 2003" }
-FINAL :: finalize_single ! { dg-error "Fortran 2003" }
- END TYPE mytype
+ CONTAINS ! { dg-error "Fortran 2003: CONTAINS block in derived type definition" }
+FINAL :: finalize_single ! { dg-error "Fortran 2003: FINAL procedure declaration|FINAL procedure 'finalize_single' at .1. is not a SUBROUTINE" }
+ END TYPE mytype ! { dg-error "Fortran 2008: Derived type definition at .1. with empty CONTAINS section" }
CONTAINS
@@ -28,6 +28,3 @@ PROGRAM finalizer
IMPLICIT NONE
! Do nothing
END PROGRAM finalizer
-
-! TODO: Remove this once finalization is implemented.
-! { dg-excess-errors "not yet implemented" }
diff --git a/gcc/testsuite/gfortran.dg/finalize_7.f03 b/gcc/testsuite/gfortran.dg/finalize_7.f03
index 6ca4f55..5807ed5 100644
--- a/gcc/testsuite/gfortran.dg/finalize_7.f03
+++ b/gcc/testsuite/gfortran.dg/finalize_7.f03
@@ -52,6 +52,3 @@ PROGRAM finalizer
IMPLICIT NONE
! Nothing here
END PROGRAM finalizer
-
-! TODO: Remove this once finalization is implemented.
-! { dg-excess-errors "not yet implemented" }
--- /dev/null 2013-05-27 09:23:19.299118255 +0200
+++ gcc/gcc/testsuite/gfortran.dg/finalize_11.f90 2013-05-27 11:59:29.426857773 +0200
@@ -0,0 +1,31 @@
+! { dg-do compile }
+! { dg-options "-std=f2003" }
+!
+! Copied from finalize_6.f90 - was before rejected as the finalization
+! wrapper uses TS29913 (-std=f2008ts) features.
+!
+
+MODULE final_type
+ IMPLICIT NONE
+
+ TYPE :: mytype
+INTEGER :: fooarr(42)
+REAL :: foobar
+ CONTAINS
+FINAL :: finalize_single
+ END TYPE mytype
+
+CONTAINS
+
+ SUBROUTINE finalize_single (el)
+IMPLICIT NONE
+TYPE(mytyp
Re: [PATCH][gensupport] Add optional attributes field to define_cond_exec
> I agree that it is possible to use define_subst (apart from the fact
> that it doesn't support define_insn_and_split yet). It's just that I
> think it looks hacky and makes the patterns look
> less readable if the reader has to remember that predication is implicit
> when a subst_attr is encountered that is tied to that particular
> define_subst.
It looks hacky indeed, but afaiu, current define_cond_exec doesn't
satisfy your needs as well - you want to add something to it, right?
So I thought that probably it's better to consider using define_subst
(as a possible replacement of define_cond_exec in some future) - and
maybe adjusting it to your needs and making it look less 'hacky', than
changing define_cond_exec.
Anyway, points you mentioned are quite reasonable, so it's all up to you:)
Thanks, Michael
On 24 May 2013 19:22, Kyrylo Tkachov wrote:
>> > As things stand now, if "predicable" is set to "no" for a
>> particular
>> > alternative, the value of control_attr is irrelevant, that
>> alternative
>> > will never have a cond_exec version. In your scheme, however,
>> > the presence of triggers the creation of
>> cond_exec
>> > variants for all of the alternatives, even the ones that we don't
>> want
>> > to cond_exec.
>> Well, that's not quite right. Internally, define_cond_exec works
>> pretty similar to define_subst. It can't be applied to one
>> alternative
>> and not applied to another - it works on the entire pattern. What
>> it
>> does to distinguish alternatives basing on 'predicable' attribute
>> is
>> to properly set attribute 'ce_enabled'.
>
> Fair enough. What I meant is that conceptually, it's like there doesn't
> exist a cond_exec variant when "predicable" is set to "no".
>
>>
>> Here is a small example (you could try it yourself by invoking
>> genmddump which is located in build directory):
>> -EXAMPLE 1-
>> (define_attr "predicable" "no,yes" (const_string "no"))
>> (define_insn "aaa"
>> [(set (match_operand:SI 0 "register_operand" "=r,m,x")
>>(match_operand:SI 1 "register_operand" "r,m,x"))]
>> ""
>> "add %0 %1"
>> [(set_attr "predicable" "yes,no,yes")])
>>
>> (define_cond_exec
>> [(match_operator 0 "arm_comparison_operator"
>> [(match_operand 1 "cc_register" "")
>> (const_int 0)])]
>> "TARGET_32BIT"
>> "")
>> - END OF EXAMPLE 1-
>>
>> And here is what the compiler has after expanding all patterns (it
>> is
>> output of genmddump):
>> ;; built-in: -1
>> (define_attr ("nonce_enabled") ("no,yes") (const_string ("yes")))
>> ;; built-in: -1
>> (define_attr ("ce_enabled") ("no,yes") (const_string ("yes")))
>> ;; a.md: 1
>> (define_attr ("predicable") ("no,yes") (const_string ("no")))
>> ;; a.md: 3
>> (define_insn ("aaa")
>> [ (set (match_operand:SI 0 ("register_operand") ("=r,m,x"))
>> (match_operand:SI 1 ("register_operand") ("r,m,x")))
>> ] ("") ("add %0 %1")
>> [ (set_attr ("predicable") ("yes,no,yes")) ])
>>
>> ;; a.md: 3
>> (define_insn ("*p aaa")
>> [ (cond_exec (match_operator 2 ("arm_comparison_operator")
>> [(match_operand 3 ("cc_register") (""))
>> (const_int 0 [0]) ])
>> (set (match_operand:SI 0 ("register_operand")
>> ("=r,m,x"))
>> (match_operand:SI 1 ("register_operand")
>> ("r,m,x"
>> ] ("TARGET_32BIT") ("add %0 %1")
>> [ (set_attr ("ce_enabled") ("yes,no,yes")) ])
>>
>> As you might see, it doesn't distinguish alternatives at all - it
>> just
>> fills 'ce_enabled' attribute with proper values.
>> Here is a second example, which is actually pretty similar to the
>> first one, but it's done with define_subst:
>> -EXAMPLE 2-
>> (define_subst_attr "at" "ce_subst" "yes" "no")
>>
>> (define_insn "aaa"
>> [(set (match_operand:SI 0 "register_operand" "=r,m,x")
>>(match_operand:SI 1 "register_operand" "r,m,x"))]
>> ""
>> "add %0 %1"
>> [(set_attr "ce_enabled" "yes,,yes")])
>
> Isn't ce_enabled an internal, implicit attribute? I don't think it was
> meant to be directly manipulated by the MD patterns.
> It seems like a hack around the canonical mechanism we have for defining
> conditional execution (by way of the "predicable" attribute).
>
> Presumably, in order to disable the cond_exec variant dynamically
> (alternative 3 in your example)
> I'd have to add:
> (define_subst_attr "subst_predicated" "ce_subst" "no" "yes")
>
> and then in the "aaa" define_insn add:
>
> (set_attr "predicated" "")
> (set_attr "control_attr" "yes,yes,no")
>
> and manipulate the "enabled" rules considering the dynamic condition,
> "predicated" and "control_attr".
> This has the disadvantages of:
> - Having to copy (set_attr "predicated" "") in all
> patterns that are predicable.
> - Using the internal ce_enabled attribute instead of the canonical
> "predicable".
> - Still doesn't work for define_insn_and_split ;)
>
>>
>> (define_subst "ce_subst"
>> [(match_operand 0)]
>> "TARGET_32BIT"
>> [(cond_exec (match_operator 1 "arm_comparison_
