| Message ID | CABGF_gcpJKNiN+Km=NOeK15j0DSS3nnhNkKD6xnzeNw3PLQ9pg@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
If I’m not mistaken all tree nodes, which have pointer type, can be divided into two groups: the type is a is a pointer to data member (TYPE_PTRMEM_P is true for it), the type is a pointer type, and the pointee is not a data member (TYPE_PTR_P is true for it). I think that we’re interested in disabling of the second group handling. It can also be divided into two groups: the type is a pointer to function type (TYPE_PTRFN_P is true for it), the type is a pointer to object type (TYPE_PTROB_P is true for it). In my opinion, the second one is worth to be considered. It contains, for example, nop_expr (these nodes are used to represent conversions that do not require any code-generation) integer_cst (these nodes represent integer constants), ssa_name. I haven’t found all types, which are contained in it. However, I think that we could disable other types, if it is necessary in the future. > What we should do later is to build a run-time check that ensures > no pointer overflow is happening and then just create code without it. I think that it is better to do this when the pointer handling is completed. I’ve attached a Change_Log, which corresponds to the previous patch. Are they fine for trunk? Could we give a headsup on the GCC mailing list and ask other people to try the new isl support in case this patch is committed?
On 13/08/2014 16:07, Roman Gareev wrote: > If I’m not mistaken all tree nodes, which have pointer type, can be > divided into two groups: the type is a is a pointer to data member > (TYPE_PTRMEM_P is true for it), the type is a pointer type, and the > pointee is not a data member (TYPE_PTR_P is true for it). I think that > we’re interested in disabling of the second group handling. It can > also be divided into two groups: the type is a pointer to function > type (TYPE_PTRFN_P is true for it), the type is a pointer to object > type (TYPE_PTROB_P is true for it). In my opinion, the second one is > worth to be considered. It contains, for example, nop_expr (these > nodes are used to represent conversions that do not require any > code-generation) integer_cst (these nodes represent integer > constants), ssa_name. I haven’t found all types, which are contained > in it. However, I think that we could disable other types, if it is > necessary in the future. > >> What we should do later is to build a run-time check that ensures >> no pointer overflow is happening and then just create code without it. > > I think that it is better to do this when the pointer handling is completed. > > I’ve attached a Change_Log, which corresponds to the previous patch. > Are they fine for trunk? Could we give a headsup on the GCC mailing > list and ask other people to try the new isl support in case this > patch is committed? Two minor thinks: - I assume you verified this passes all graphite tests. - Please add a brief comment in the source code regarding why we do not want to detect such SCoPs. Otherwise LGTM. Cheers, Tobias
On Wed, Aug 13, 2014 at 10:07 AM, Roman Gareev <gareevroman@gmail.com> wrote: > If I’m not mistaken all tree nodes, which have pointer type, can be > divided into two groups: the type is a is a pointer to data member > (TYPE_PTRMEM_P is true for it), the type is a pointer type, and the > pointee is not a data member (TYPE_PTR_P is true for it). Both are C++ frontend concepts and not relevant for the middle-end and thus GRAPHITE. I think that > we’re interested in disabling of the second group handling. It can > also be divided into two groups: the type is a pointer to function > type (TYPE_PTRFN_P is true for it), the type is a pointer to object > type (TYPE_PTROB_P is true for it). In my opinion, the second one is > worth to be considered. It contains, for example, nop_expr (these > nodes are used to represent conversions that do not require any > code-generation) integer_cst (these nodes represent integer > constants), ssa_name. I haven’t found all types, which are contained > in it. However, I think that we could disable other types, if it is > necessary in the future. > >> What we should do later is to build a run-time check that ensures >> no pointer overflow is happening and then just create code without it. I think you can assume that pointers don't overflow. Richard. > I think that it is better to do this when the pointer handling is completed. > > I’ve attached a Change_Log, which corresponds to the previous patch. > Are they fine for trunk? Could we give a headsup on the GCC mailing > list and ask other people to try the new isl support in case this > patch is committed? > > -- > Cheers, Roman Gareev.
> - I assume you verified this passes all graphite tests. Yes, I’ve found out that there is no regression. > - Please add a brief comment in the source code regarding why we > do not want to detect such SCoPs. Would you add anything to the following comment: “We disable the handling of pointer types, because it’s currently not supported by Graphite with the ISL AST generator. SSA_NAME nodes are the only nodes, which are disabled in case they are pointers to object types, but this can be changed.” ?
Thank you for the comment! 2014-08-13 15:55 GMT+06:00 Richard Biener <richard.guenther@gmail.com>: > On Wed, Aug 13, 2014 at 10:07 AM, Roman Gareev <gareevroman@gmail.com> wrote: >> If I’m not mistaken all tree nodes, which have pointer type, can be >> divided into two groups: the type is a is a pointer to data member >> (TYPE_PTRMEM_P is true for it), the type is a pointer type, and the >> pointee is not a data member (TYPE_PTR_P is true for it). > > Both are C++ frontend concepts and not relevant for the middle-end > and thus GRAPHITE. > > I think that >> we’re interested in disabling of the second group handling. It can >> also be divided into two groups: the type is a pointer to function >> type (TYPE_PTRFN_P is true for it), the type is a pointer to object >> type (TYPE_PTROB_P is true for it). In my opinion, the second one is >> worth to be considered. It contains, for example, nop_expr (these >> nodes are used to represent conversions that do not require any >> code-generation) integer_cst (these nodes represent integer >> constants), ssa_name. I haven’t found all types, which are contained >> in it. However, I think that we could disable other types, if it is >> necessary in the future. >> >>> What we should do later is to build a run-time check that ensures >>> no pointer overflow is happening and then just create code without it. > > I think you can assume that pointers don't overflow. > > Richard. > >> I think that it is better to do this when the pointer handling is completed. >> >> I’ve attached a Change_Log, which corresponds to the previous patch. >> Are they fine for trunk? Could we give a headsup on the GCC mailing >> list and ask other people to try the new isl support in case this >> patch is committed? >> >> -- >> Cheers, Roman Gareev.
Index: gcc/graphite-scop-detection.c =================================================================== --- gcc/graphite-scop-detection.c (revision 213773) +++ gcc/graphite-scop-detection.c (working copy) @@ -54,6 +54,7 @@ #include "tree-pass.h" #include "sese.h" #include "tree-ssa-propagate.h" +#include "cp/cp-tree.h" #ifdef HAVE_cloog #include "graphite-poly.h" @@ -217,6 +218,9 @@ if (chrec_contains_undetermined (scev)) return false; + if (TYPE_PTROB_P (TREE_TYPE (scev)) && TREE_CODE (scev) == SSA_NAME) + return false; + switch (TREE_CODE (scev)) { case NEGATE_EXPR: