Re: Failing in generated file options.c

[Andrew Pinski via Gcc]

On Mon, Mar 15, 2021 at 7:41 PM Gary Oblock via Gcc  wrote:
>
> Guys,
>
> I checked out a fresh copy of the GCC sources today, applied somebodies
> patch to it and voila!
>
> options.c:13591:2: error: #error Report option property is dropped #error 
> Report option property is dropped
>
> I built this the same minimally convoluted way that I always do.
>
> cd $1
> BASE=`pwd`
> echo BASE = $BASE
> touch objdir install
> rm -rf objdir install
> mkdir objdir install
> cd objdir
> echo BUILDING IN `pwd`
> ../sources/configure --prefix=$BASE/install --disable-bootstrap 
> -enable-language=c,c++,lto --disable-multilib --enable-valgrind-annotations
> make CFLAGS='-O2 -g' CXXFLAGS='-O2 -g' -j 12
> make install
>
> The file option.c is generated in objdir/gcc by an awk script:
>
> mawk -f ../../sources/gcc/opt-functions.awk -f ../../sources/gcc/opt-read.awk 
> \
>-f ../../sources/gcc/optc-gen.awk \
>-v header_name="config.h system.h coretypes.h options.h tm.h" < 
> optionlist > options.c
>
> Does anyone  have any idea what's going to here?


# If FLAGS contains a "NAME(...argument...)" flag, return the value
# of the argument.  Print error message otherwise.

It could be any of the following:
Name/Type inside Enum
Enum/String/Value instead EnumValue

Thanks,
Andrew

>
>
> CONFIDENTIALITY NOTICE: This e-mail message, including any attachments, is 
> for the sole use of the intended recipient(s) and contains information that 
> is confidential and proprietary to Ampere Computing or its subsidiaries. It 
> is to be used solely for the purpose of furthering the parties' business 
> relationship. Any unauthorized review, copying, or distribution of this email 
> (or any attachments thereto) is strictly prohibited. If you are not the 
> intended recipient, please contact the sender immediately and permanently 
> delete the original and any copies of this email and any attachments thereto.

Failing in generated file options.c

[Erick Ochoa via Gcc]

Hi Gary,

the options.c file is generated from the gcc/common.opt file. Report was a
keyword that could be given to optimization options but which was dropped
sometime in December I think. The patches I sent you should have the
keyword Report dropped. Are you applying your sources already? If not, are
you only applying a subset of the patches? If you look at the patch
numbered 9, you will see that the Report keyword was removed. If you are
only applying patches before the 9th, yeah I would expect that error to
trigger.

-Erick

Re: Failing in generated file options.c

[Martin Liška]

On 3/16/21 3:39 AM, Gary Oblock via Gcc wrote:

Guys,

I checked out a fresh copy of the GCC sources today, applied somebodies
patch to it and voila!

options.c:13591:2: error: #error Report option property is dropped #error 
Report option property is dropped

I built this the same minimally convoluted way that I always do.

cd $1
BASE=`pwd`
echo BASE = $BASE
touch objdir install
rm -rf objdir install
mkdir objdir install
cd objdir
echo BUILDING IN `pwd`
../sources/configure --prefix=$BASE/install --disable-bootstrap 
-enable-language=c,c++,lto --disable-multilib --enable-valgrind-annotations
make CFLAGS='-O2 -g' CXXFLAGS='-O2 -g' -j 12
make install

The file option.c is generated in objdir/gcc by an awk script:

mawk -f ../../sources/gcc/opt-functions.awk -f ../../sources/gcc/opt-read.awk \
-f ../../sources/gcc/optc-gen.awk \
-v header_name="config.h system.h coretypes.h options.h tm.h" < optionlist 
> options.c

Does anyone  have any idea what's going to here?


Hey.

As already mentioned, the error message appeared in 
5137d1ae6a1fe4a3ff8b5983f6e4d9aeb69e5486:

Remove Report keyword for options

Since g:7caa49706316e650fb67719e1a1bf3a35054b685 the option is ignored

as we print used command line for -fverbose-asm output.

gcc/ChangeLog:

* doc/options.texi: Remove Report keyword.

* opt-functions.awk: Print error when Report keyword
is used.
* optc-gen.awk: Do not handle Report keyword.
* opts.h (struct cl_option): Remove cl_report bitfield flag.




CONFIDENTIALITY NOTICE: This e-mail message, including any attachments, is for 
the sole use of the intended recipient(s) and contains information that is 
confidential and proprietary to Ampere Computing or its subsidiaries. It is to 
be used solely for the purpose of furthering the parties' business 
relationship. Any unauthorized review, copying, or distribution of this email 
(or any attachments thereto) is strictly prohibited. If you are not the 
intended recipient, please contact the sender immediately and permanently 
delete the original and any copies of this email and any attachments thereto.



Small comment about this one. Is it really appropriate attaching such a notice 
if you send an email to a public mailing list?

Cheers,
Martin

Re: Failing in generated file options.c

[Jonathan Wakely via Gcc]

On Tue, 16 Mar 2021, 08:53 Martin Liška,  wrote:

> On 3/16/21 3:39 AM, Gary Oblock via Gcc wrote:
> >
> >
> > CONFIDENTIALITY NOTICE: This e-mail message, including any attachments,
> is for the sole use of the intended recipient(s) and contains information
> that is confidential and proprietary to Ampere Computing or its
> subsidiaries. It is to be used solely for the purpose of furthering the
> parties' business relationship. Any unauthorized review, copying, or
> distribution of this email (or any attachments thereto) is strictly
> prohibited. If you are not the intended recipient, please contact the
> sender immediately and permanently delete the original and any copies of
> this email and any attachments thereto.
> >
>
> Small comment about this one. Is it really appropriate attaching such a
> notice if you send an email to a public mailing list?
>

Our policies say it's not appropriate:
https://gcc.gnu.org/lists.html#policies


>
>

printf: Ambiguous warning

[Rene Kita]

(Please keep me CC'd, I'm not subscribe to the list)

Here is a minimal example:
#include 

int
main()
{
  unsigned short n;
  unsigned short *p;
  p = &n;

  printf("p: %hn\n", p);
}


% gcc -Wall -Wpedantic main.c
main.c: In function 'main':
main.c:10:16: warning: format '%hn' expects argument of type 'short int *', but 
argument 2 has type 'short unsigned int *' [-Wformat=]
   10 |   printf("p: %hn\n", p);
  |  ~~^ ~
  || |
  || short unsigned int *
  |short int *
  |  %hn

The warning for line 10 suggests to use '%hn' as format specifier which
is already used and the wrong one. AFAIK the correct format specifier
would be '%p' here.

I didn't not find a bug report for this. Is this a known problem? Should
I file a bug report for this?


% gcc -v
Using built-in specs.
COLLECT_GCC=gcc
COLLECT_LTO_WRAPPER=/home/kt/bin/gcc-latest/libexec/gcc/x86_64-pc-linux-gnu/11.0.1/lto-wrapper
Target: x86_64-pc-linux-gnu
Configured with: ../configure --disable-multilib --disable-werror
--enable-languages=c,c++,go --disable-bootstrap --disable-nls
--enable-threads=posix --enable-default-pie --enable-checking=release
--program-suffix=-latest --prefix=/home/kt/bin/gcc-latest
Thread model: posix
Supported LTO compression algorithms: zlib zstd
gcc version 11.0.1 20210315 (experimental) (GCC) 

Re: printf: Ambiguous warning

[Jakub Jelinek via Gcc]

On Tue, Mar 16, 2021 at 11:20:05AM +0100, Rene Kita wrote:
> (Please keep me CC'd, I'm not subscribe to the list)
> 
> Here is a minimal example:
> #include 
> 
> int
> main()
> {
>   unsigned short n;
>   unsigned short *p;
>   p = &n;
> 
>   printf("p: %hn\n", p);
> }
> 
> 
> % gcc -Wall -Wpedantic main.c
> main.c: In function 'main':
> main.c:10:16: warning: format '%hn' expects argument of type 'short int *', 
> but argument 2 has type 'short unsigned int *' [-Wformat=]
>10 |   printf("p: %hn\n", p);
>   |  ~~^ ~
>   || |
>   || short unsigned int *
>   |short int *
>   |  %hn
> 
> The warning for line 10 suggests to use '%hn' as format specifier which
> is already used and the wrong one. AFAIK the correct format specifier
> would be '%p' here.

No, the warning tells you that argument for %hn should have short int *
type, not unsigned short int *.
C99 says for n:
"The argument shall be a pointer to signed integer into which is written the
number of characters written to the output stream so far by this call to
fprintf. No argument is converted, but one is consumed. If the conversion
specification includes any flags, a field width, or a precision, the behavior is
undefined."
Note the SIGNED in there.  And for the h modifier:
"or that a following n conversion specifier applies to a pointer to a short
int argument."
(not unsigned short int).

> I didn't not find a bug report for this. Is this a known problem? Should
> I file a bug report for this?

No, this is a bug in your testcase that is correctly diagnosed.

Jakub

Re: printf: Ambiguous warning

[Rene Kita]

On Tue, Mar 16, 2021 at 11:26:29AM +0100, Jakub Jelinek wrote:
> On Tue, Mar 16, 2021 at 11:20:05AM +0100, Rene Kita wrote:
> > % gcc -Wall -Wpedantic main.c
> > main.c: In function 'main':
> > main.c:10:16: warning: format '%hn' expects argument of type 'short int *', 
> > but argument 2 has type 'short unsigned int *' [-Wformat=]
> >10 |   printf("p: %hn\n", p);
> >   |  ~~^ ~
> >   || |
> >   || short unsigned int *
> >   |short int *
> >   |  %hn
^

> > The warning for line 10 suggests to use '%hn' as format specifier which
> > is already used and the wrong one. AFAIK the correct format specifier
> > would be '%p' here.
> 
> No, the warning tells you that argument for %hn should have short int *
> type, not unsigned short int *.
I understand this and I don't say the warning is wrong but the suggested
solution. I have highlighted the part of the output I'm talking about
above. If you replace the '%hn' with e.g. '%d' you get the same
suggestion:

main.c: In function 'main':
main.c:10:15: warning: format '%d' expects argument of type 'int', but argument 
2 has type 'short unsigned int *' [-Wformat=]
   10 |   printf("p: %d\n", p);
  |  ~^ ~
  |   | |
  |   int   short unsigned int *
  |  %hn
^

>   Jakub
Rene

GCC 11.0.1 Status Report (2021-03-16)

[Richard Biener]

Status
==

GCC trunk which eventually will become GCC 11 is still in
regression and documentation fixes only mode (Stage 4).

If history should repeat itself then a first release candidate
of GCC 11 will be released mid April.  For this to happen
we need to resolve the remaining 17 P1 regressions - 8 of which
are classified as target or bootstrap issues.  Please have
a look at the set of P1 (and preferably also P2) regressions
which can be conveniently accessed via the 'Serious regressions'
link on https://gcc.gnu.org


Quality Data


Priority  #   Change from last report
---   ---
P1   17   - 45
P2  288   - 46
P3   37   +  1
P4  192   +  2
P5   24
---   ---
Total P1-P3 342   - 90
Total   558   - 88


Previous Report
===

https://gcc.gnu.org/pipermail/gcc/2021-January/234703.html

Re: printf: Ambiguous warning

[Jonathan Wakely via Gcc]

On Tue, 16 Mar 2021 at 10:47, Rene Kita  wrote:
>
> On Tue, Mar 16, 2021 at 11:26:29AM +0100, Jakub Jelinek wrote:
> > On Tue, Mar 16, 2021 at 11:20:05AM +0100, Rene Kita wrote:
> > > % gcc -Wall -Wpedantic main.c
> > > main.c: In function 'main':
> > > main.c:10:16: warning: format '%hn' expects argument of type 'short int 
> > > *', but argument 2 has type 'short unsigned int *' [-Wformat=]
> > >10 |   printf("p: %hn\n", p);
> > >   |  ~~^ ~
> > >   || |
> > >   || short unsigned int *
> > >   |short int *
> > >   |  %hn
> ^
>
> > > The warning for line 10 suggests to use '%hn' as format specifier which
> > > is already used and the wrong one. AFAIK the correct format specifier
> > > would be '%p' here.
> >
> > No, the warning tells you that argument for %hn should have short int *
> > type, not unsigned short int *.
> I understand this and I don't say the warning is wrong but the suggested
> solution. I have highlighted the part of the output I'm talking about
> above. If you replace the '%hn' with e.g. '%d' you get the same
> suggestion:
>
> main.c: In function 'main':
> main.c:10:15: warning: format '%d' expects argument of type 'int', but 
> argument 2 has type 'short unsigned int *' [-Wformat=]
>10 |   printf("p: %d\n", p);
>   |  ~^ ~
>   |   | |
>   |   int   short unsigned int *
>   |  %hn
> ^

This looks similar to https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98819

Question on special constraint variables in points-to analysis

[Erick Ochoa via Gcc]

Hello,

I'm currently working on improving my understanding of the implementation
of the intraprocedural points-to analysis in GCC. I have already read the
papers by Daniel Berlin and have been looking at the source for some time,
but I understand that those references are a little bit old and might not
fully reflect the status of the current points-to analysis.

In order to familiarize myself with the analysis I decided to take a look
at the constraints dumped in the logs and input them into a different
solver which also implements a points-to analysis. I do this during GCC's
run time. In other words, before GCC computes the points-to analysis using
the regular intraprocedural points-to analysis, I take a look at the
constraint vector and translate them to an interface the other solver can
understand. Therefore, after this other solver finished, there are two sets
of solutions for the points-to analysis.

In order to verify the correctness of my analysis I simply iterate over
GCC's solution and place an assertion to verify that both points-to sets
for each variable are equal. The solution computed by my implementation
solves the constraints by abstracting the varinfo into integers by using
the varinfo id. And then the relationship between points-to variables are
expressed as variable A id points to variable B id. Since the variable info
ids are just offsets into the varinfo array, then there should be a one to
one mapping between the solutions in this solver to the solutions computed
by GCC.

Its implemented something like the following:

```
for (auto output : points_to_analysis) {
   int from_i; // pointer variable index in get_varinfo(from_i)
   int to_i; // point-ee variable index in get_varinfo(toi)

   if(!get_varinfo(from_i)->may_have_pointers) continue;
   bitmap vars = get_varinfo(from_i)->solution;
   if (!vars) continue;

   if (dump_file) fprintf(dump_file, "%s -> %s\n",
get_varinfo(from_i)->name, get_varinfo(to_i)->name);

   bool same = bitmap_bit_p(vars, to_i);
   if (dump_file) fprintf(dump_file, "SAME = %s\n", same ? "true" :
"false");
   gcc_assert(same);

}
```

This means that if the two analyses were equal, all test cases testing
-ftree-pta should succeed. At the moment, the assertion is being triggered.
Now, I would like to understand why the assertion is triggered without
asking you to debug my code, so instead I'm trying to understand why GCC's
solution is correct. (I gave all this exposition because I may be making
assumptions that are incorrect and that can quickly be verified by someone
more knowledgeable.) Now, I'll focus on the test case:

The code in question is a slightly modified version of the test case
ipa-pta-16.c
```
struct X
{
  long l1;
  struct Y
{
  long l2;
  int *p;
} y;
};
int i;
static int __attribute__((noinline))
foo (struct X *x)
{
  struct Y y = x->y;
  *y.p = 0;
  i = 1;
  return *y.p;
}
extern void abort (void);
int main()
{
  struct X x;
  struct X t;
  x.y.p = &i;
  long* e = &t.l1;
  if (foo(&x) != 1)
abort ();
  return e;
}
```

and is being compiled with the following flags:

gcc -O2 -ftree-pta -ftree-pta-external-solver -fdump-tree-alias
-ftree-salias ipa-pta-16.c

I would like to focus on the foo function, which is translated to the
following gimple:
```
__attribute__((noinline))
int foo.constprop.isra (int * ISRA.4)
{
  int * y$p;
  struct X * x;
  struct X * x;
  int _2;

   [local count: 1073741824]:
  *ISRA.4_3(D) = 0;
  i = 1;
  _2 = *ISRA.4_3(D);
  return _2;

}
```

GCC generates the following constraints (as seen on the dump):

```
ANYTHING = &ANYTHING
ESCAPED = *ESCAPED
ESCAPED = ESCAPED + UNKNOWN
*ESCAPED = NONLOCAL
NONLOCAL = &NONLOCAL
NONLOCAL = &ESCAPED
INTEGER = &ANYTHING
ISRA.4 = &NONLOCAL
derefaddrtmp(9) = &NULL
*ISRA.4 = derefaddrtmp(9)
i = NONLOCAL
i = &NONLOCAL
ESCAPED = &NONLOCAL
_2 = *ISRA.4
```

The points-to set generated by GCC are the following:

```
ANYTHING = { ANYTHING }
ESCAPED = { NULL ESCAPED NONLOCAL }
NONLOCAL = { ESCAPED NONLOCAL } same as i
STOREDANYTHING = { }
INTEGER = { ANYTHING }
ISRA.4 = { NONLOCAL }
derefaddrtmp(9) = { NULL }
i = { ESCAPED NONLOCAL }
_2 = { ESCAPED NONLOCAL }
foo.constprop.0.isra.0 = { }
```

I also noticed that there is more information about ISRA.4 which is not
printed in the points-to sets printed above. Instead, the dump continues
and adds this line:

```
Flow-insensitive points-to information

ISRA.4_3(D), points-to non-local, points-to NULL, points-to vars: { }
```

Now, I understand that the first set of solutions (i.e, `{ NONLOCAL }`) is
the solution of the constraint variable. That is, the constraint variable
ISRA.4 when assigned the set `{ NONLOCAL }` is a solution to the
constraints... I also understand that ISRA.4_3(D) is the SSA variable on
the gimple code. There is technically a mapping between the two, but I
guess there's some distinction. I also understand that the first dump is
generated from the function dump_so

Re: Question on special constraint variables in points-to analysis

[Richard Biener via Gcc]

On Tue, Mar 16, 2021 at 1:16 PM Erick Ochoa via Gcc  wrote:
>
> Hello,
>
> I'm currently working on improving my understanding of the implementation
> of the intraprocedural points-to analysis in GCC. I have already read the
> papers by Daniel Berlin and have been looking at the source for some time,
> but I understand that those references are a little bit old and might not
> fully reflect the status of the current points-to analysis.
>
> In order to familiarize myself with the analysis I decided to take a look
> at the constraints dumped in the logs and input them into a different
> solver which also implements a points-to analysis. I do this during GCC's
> run time. In other words, before GCC computes the points-to analysis using
> the regular intraprocedural points-to analysis, I take a look at the
> constraint vector and translate them to an interface the other solver can
> understand. Therefore, after this other solver finished, there are two sets
> of solutions for the points-to analysis.
>
> In order to verify the correctness of my analysis I simply iterate over
> GCC's solution and place an assertion to verify that both points-to sets
> for each variable are equal. The solution computed by my implementation
> solves the constraints by abstracting the varinfo into integers by using
> the varinfo id. And then the relationship between points-to variables are
> expressed as variable A id points to variable B id. Since the variable info
> ids are just offsets into the varinfo array, then there should be a one to
> one mapping between the solutions in this solver to the solutions computed
> by GCC.
>
> Its implemented something like the following:
>
> ```
> for (auto output : points_to_analysis) {
>int from_i; // pointer variable index in get_varinfo(from_i)
>int to_i; // point-ee variable index in get_varinfo(toi)
>
>if(!get_varinfo(from_i)->may_have_pointers) continue;
>bitmap vars = get_varinfo(from_i)->solution;
>if (!vars) continue;
>
>if (dump_file) fprintf(dump_file, "%s -> %s\n",
> get_varinfo(from_i)->name, get_varinfo(to_i)->name);
>
>bool same = bitmap_bit_p(vars, to_i);
>if (dump_file) fprintf(dump_file, "SAME = %s\n", same ? "true" :
> "false");
>gcc_assert(same);
>
> }
> ```
>
> This means that if the two analyses were equal, all test cases testing
> -ftree-pta should succeed. At the moment, the assertion is being triggered.
> Now, I would like to understand why the assertion is triggered without
> asking you to debug my code, so instead I'm trying to understand why GCC's
> solution is correct. (I gave all this exposition because I may be making
> assumptions that are incorrect and that can quickly be verified by someone
> more knowledgeable.) Now, I'll focus on the test case:
>
> The code in question is a slightly modified version of the test case
> ipa-pta-16.c
> ```
> struct X
> {
>   long l1;
>   struct Y
> {
>   long l2;
>   int *p;
> } y;
> };
> int i;
> static int __attribute__((noinline))
> foo (struct X *x)
> {
>   struct Y y = x->y;
>   *y.p = 0;
>   i = 1;
>   return *y.p;
> }
> extern void abort (void);
> int main()
> {
>   struct X x;
>   struct X t;
>   x.y.p = &i;
>   long* e = &t.l1;
>   if (foo(&x) != 1)
> abort ();
>   return e;
> }
> ```
>
> and is being compiled with the following flags:
>
> gcc -O2 -ftree-pta -ftree-pta-external-solver -fdump-tree-alias
> -ftree-salias ipa-pta-16.c
>
> I would like to focus on the foo function, which is translated to the
> following gimple:
> ```
> __attribute__((noinline))
> int foo.constprop.isra (int * ISRA.4)
> {
>   int * y$p;
>   struct X * x;
>   struct X * x;
>   int _2;
>
>[local count: 1073741824]:
>   *ISRA.4_3(D) = 0;
>   i = 1;
>   _2 = *ISRA.4_3(D);
>   return _2;
>
> }
> ```
>
> GCC generates the following constraints (as seen on the dump):
>
> ```
> ANYTHING = &ANYTHING
> ESCAPED = *ESCAPED
> ESCAPED = ESCAPED + UNKNOWN
> *ESCAPED = NONLOCAL
> NONLOCAL = &NONLOCAL
> NONLOCAL = &ESCAPED
> INTEGER = &ANYTHING
> ISRA.4 = &NONLOCAL
> derefaddrtmp(9) = &NULL
> *ISRA.4 = derefaddrtmp(9)
> i = NONLOCAL
> i = &NONLOCAL
> ESCAPED = &NONLOCAL
> _2 = *ISRA.4
> ```
>
> The points-to set generated by GCC are the following:
>
> ```
> ANYTHING = { ANYTHING }
> ESCAPED = { NULL ESCAPED NONLOCAL }
> NONLOCAL = { ESCAPED NONLOCAL } same as i
> STOREDANYTHING = { }
> INTEGER = { ANYTHING }
> ISRA.4 = { NONLOCAL }
> derefaddrtmp(9) = { NULL }
> i = { ESCAPED NONLOCAL }
> _2 = { ESCAPED NONLOCAL }
> foo.constprop.0.isra.0 = { }
> ```
>
> I also noticed that there is more information about ISRA.4 which is not
> printed in the points-to sets printed above. Instead, the dump continues
> and adds this line:
>
> ```
> Flow-insensitive points-to information
>
> ISRA.4_3(D), points-to non-local, points-to NULL, points-to vars: { }
> ```
>
> Now, I understand that the first set of solutions (i.e, `{ NONLOCAL }`) is
> the solution of the constrain

Re: 'walk_stmt_load_store_addr_ops' for non-'gimple_assign_single_p (stmt)'

[Thomas Schwinge]

Hi!

Richard, thanks for your answer.  I'll need to look into this more; two
questions already:

On 2021-03-15T20:17:17+0100, Richard Biener via Gcc  wrote:
> On March 15, 2021 7:31:46 PM GMT+01:00, Thomas Schwinge 
>  wrote:
>>First time I'm using this API -- so the error certainly may be on my
>>side.  ;-)
>>
>>What I'm doing, is a 'walk_gimple_seq', and in that one's
>>'callback_stmt', call 'walk_stmt_load_store_addr_ops', to collect
>>variable load/store/address-taken instances.  This did seem quite
>>straight-forward, given the description; 'gcc/gimple-walk.c':
>>
>>/* For the statement STMT call the callbacks VISIT_LOAD, VISIT_STORE
>>and
>>  VISIT_ADDR if non-NULL on loads, store and address-taken operands
>>passing the STMT, the base of the operand, the operand itself
>>containing
>>  the base and DATA to it.  The base will be either a decl, an indirect
>> reference (including TARGET_MEM_REF) or the argument of an address
>>   expression.
>>   Returns the results of these callbacks or'ed.  */
>>
>>bool
>>walk_stmt_load_store_addr_ops (gimple *stmt, void *data,
>>   walk_stmt_load_store_addr_fn visit_load,
>>  walk_stmt_load_store_addr_fn visit_store,
>>   walk_stmt_load_store_addr_fn visit_addr)
>>{ [...] }
>>
>>Indeed, given (Fortran) 'zzz = 1', we produce GIMPLE:
>>
>>gimple_assign 
>>
>>..., and calling 'walk_stmt_load_store_addr_ops' on that, I see, as
>>expected, the 'visit_store' callback invoked, with 'rhs' and 'arg':
>>''.
>>
>>However, given (Fortran) 'zzz = r + r2', we produce GIMPLE:
>>
>>gimple_assign 
>>
>>..., and calling 'walk_stmt_load_store_addr_ops' on that, I see,
>>unexpectedly, no callback at all invoked: neither 'visit_load', nor
>>'visit_store' (nor 'visit_address', obviously).
>
> The variables involved are registers. You only get called on memory operands.

How would I have told that from the 'walk_stmt_load_store_addr_ops'
function description?  (How to improve that one "to reflect relatity"?)

But 'zzz' surely is the same in 'zzz = 1' vs. 'zzz = r + r2' -- for the
former I *do* see the 'visit_store' callback invoked, for the latter I
don't?

>>From a quick look at 'gcc/gimple-walk.c:walk_stmt_load_store_addr_ops',
>>this seems to intentionally be implemented in this way -- but I don't
>>understand the rationale?
>>
>>
>>Instead of 'walk_gimple_seq' -> 'callback_stmt' ->
>>'walk_stmt_load_store_addr_ops', do I need to use 'walk_gimple_seq' ->
>>'callback_op' -> "something"?
>
> Yes, if you want to visit register sets and uses as a well. Note you'll also 
> see constants that way.

I'll look into that; in particular to figure out "something" for what I
need: load/store/address-taken.


Grüße
 Thomas
-
Mentor Graphics (Deutschland) GmbH, Arnulfstrasse 201, 80634 München 
Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Frank 
Thürauf

Re: 'walk_stmt_load_store_addr_ops' for non-'gimple_assign_single_p (stmt)'

[Richard Biener via Gcc]

On Tue, Mar 16, 2021 at 4:02 PM Thomas Schwinge  wrote:
>
> Hi!
>
> Richard, thanks for your answer.  I'll need to look into this more; two
> questions already:
>
> On 2021-03-15T20:17:17+0100, Richard Biener via Gcc  wrote:
> > On March 15, 2021 7:31:46 PM GMT+01:00, Thomas Schwinge 
> >  wrote:
> >>First time I'm using this API -- so the error certainly may be on my
> >>side.  ;-)
> >>
> >>What I'm doing, is a 'walk_gimple_seq', and in that one's
> >>'callback_stmt', call 'walk_stmt_load_store_addr_ops', to collect
> >>variable load/store/address-taken instances.  This did seem quite
> >>straight-forward, given the description; 'gcc/gimple-walk.c':
> >>
> >>/* For the statement STMT call the callbacks VISIT_LOAD, VISIT_STORE
> >>and
> >>  VISIT_ADDR if non-NULL on loads, store and address-taken operands
> >>passing the STMT, the base of the operand, the operand itself
> >>containing
> >>  the base and DATA to it.  The base will be either a decl, an indirect
> >> reference (including TARGET_MEM_REF) or the argument of an address
> >>   expression.
> >>   Returns the results of these callbacks or'ed.  */
> >>
> >>bool
> >>walk_stmt_load_store_addr_ops (gimple *stmt, void *data,
> >>   walk_stmt_load_store_addr_fn visit_load,
> >>  walk_stmt_load_store_addr_fn visit_store,
> >>   walk_stmt_load_store_addr_fn visit_addr)
> >>{ [...] }
> >>
> >>Indeed, given (Fortran) 'zzz = 1', we produce GIMPLE:
> >>
> >>gimple_assign 
> >>
> >>..., and calling 'walk_stmt_load_store_addr_ops' on that, I see, as
> >>expected, the 'visit_store' callback invoked, with 'rhs' and 'arg':
> >>''.
> >>
> >>However, given (Fortran) 'zzz = r + r2', we produce GIMPLE:
> >>
> >>gimple_assign 
> >>
> >>..., and calling 'walk_stmt_load_store_addr_ops' on that, I see,
> >>unexpectedly, no callback at all invoked: neither 'visit_load', nor
> >>'visit_store' (nor 'visit_address', obviously).
> >
> > The variables involved are registers. You only get called on memory 
> > operands.
>
> How would I have told that from the 'walk_stmt_load_store_addr_ops'
> function description?  (How to improve that one "to reflect relatity"?)

Hmm, from the function name which mentions 'load' and 'store'? ;)

For example we have

static inline bool
gimple_store_p (const gimple *gs)
{
  tree lhs = gimple_get_lhs (gs);
  return lhs && !is_gimple_reg (lhs);
}


> But 'zzz' surely is the same in 'zzz = 1' vs. 'zzz = r + r2' -- for the
> former I *do* see the 'visit_store' callback invoked, for the latter I
> don't?
>
> >>From a quick look at 'gcc/gimple-walk.c:walk_stmt_load_store_addr_ops',
> >>this seems to intentionally be implemented in this way -- but I don't
> >>understand the rationale?
> >>
> >>
> >>Instead of 'walk_gimple_seq' -> 'callback_stmt' ->
> >>'walk_stmt_load_store_addr_ops', do I need to use 'walk_gimple_seq' ->
> >>'callback_op' -> "something"?
> >
> > Yes, if you want to visit register sets and uses as a well. Note you'll 
> > also see constants that way.
>
> I'll look into that; in particular to figure out "something" for what I
> need: load/store/address-taken.

Well, indeed.  The question is what you are doing the exercise for ;)
(and thus what you actually need visited)

Richard.

>
> Grüße
>  Thomas
> -
> Mentor Graphics (Deutschland) GmbH, Arnulfstrasse 201, 80634 München 
> Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Frank 
> Thürauf

Re: 'walk_stmt_load_store_addr_ops' for non-'gimple_assign_single_p (stmt)'

[Michael Matz]

Hello,

On Tue, 16 Mar 2021, Thomas Schwinge wrote:

> >>Indeed, given (Fortran) 'zzz = 1', we produce GIMPLE:
> >>
> >>gimple_assign 
> >>
> >>..., and calling 'walk_stmt_load_store_addr_ops' on that, I see, as
> >>expected, the 'visit_store' callback invoked, with 'rhs' and 'arg':
> >>''.
> >>
> >>However, given (Fortran) 'zzz = r + r2', we produce GIMPLE:
> >>
> >>gimple_assign 

But that's pre-ssa form.  After writing into SSA 'zzz' will be replaced by 
an SSA name, and the actual store into 'zzz' will happen in a store 
instruction.

> >>..., and calling 'walk_stmt_load_store_addr_ops' on that, I see,
> >>unexpectedly, no callback at all invoked: neither 'visit_load', nor
> >>'visit_store' (nor 'visit_address', obviously).
> >
> > The variables involved are registers. You only get called on memory 
> > operands.
> 
> How would I have told that from the 'walk_stmt_load_store_addr_ops'
> function description?  (How to improve that one "to reflect relatity"?)
> 
> But 'zzz' surely is the same in 'zzz = 1' vs. 'zzz = r + r2' -- for the
> former I *do* see the 'visit_store' callback invoked, for the latter I
> don't?

The walk_gimple functions are intended to be used on the SSA form of 
gimple (i.e. the one that it is in most of the time).  And in that it's 
not the case that 'zzz = 1' and 'zzz = r + r2' are similar.  The former 
can have memory as the lhs (that includes static variables, or indirection 
through pointers), the latter can not.  The lhs of a binary statement is 
always an SSA name.  A write to an SSA name is not a store, which is why 
it's not walked for walk_stmt_load_store_addr_ops.

Maybe it helps to look at simple C examples:

% cat x.c
int zzz;
void foo(void) { zzz = 1; }
void bar(int i) { zzz = i + 1; }
% gcc -c x.c -fdump-tree-ssa-vops
% cat x.c.*ssa
foo ()
{
   :
  # .MEM_2 = VDEF <.MEM_1(D)>
  zzz = 1;
  # VUSE <.MEM_2>
  return;
}

bar (int i)
{
  int _1;

   :
  _1 = i_2(D) + 1;
  # .MEM_4 = VDEF <.MEM_3(D)>
  zzz = _1;
  # VUSE <.MEM_4>
  return;

}

See how the instruction writing to zzz (a global, and hence memory) is 
going through a temporary for the addition in bar?  This will always be 
the case when the expression is arithmetic.

In SSA form gimple only very few instruction types can be stores, namely 
calls and stores like above (where the RHS is an unary tree).  If you want 
to capture writes into SSA names as well (which are more appropriately 
thought of as 'setting the ssa name' or 'associating the ssa name with the 
rhs value') you need the per-operand callback indeed.  But that depends on 
what you actually want to do.


Ciao,
Michael.

Re: 'walk_stmt_load_store_addr_ops' for non-'gimple_assign_single_p (stmt)'

[Thomas Schwinge]

Hi!

Thanks, Michael, and again Richard for your quick responses.

On 2021-03-16T15:25:10+, Michael Matz  wrote:
> On Tue, 16 Mar 2021, Thomas Schwinge wrote:
>
>> >>Indeed, given (Fortran) 'zzz = 1', we produce GIMPLE:
>> >>
>> >>gimple_assign 
>> >>
>> >>..., and calling 'walk_stmt_load_store_addr_ops' on that, I see, as
>> >>expected, the 'visit_store' callback invoked, with 'rhs' and 'arg':
>> >>''.
>> >>
>> >>However, given (Fortran) 'zzz = r + r2', we produce GIMPLE:
>> >>
>> >>gimple_assign 
>
> But that's pre-ssa form.  After writing into SSA 'zzz' will be replaced by
> an SSA name, and the actual store into 'zzz' will happen in a store
> instruction.

Oh, I see, and...

>> >>..., and calling 'walk_stmt_load_store_addr_ops' on that, I see,
>> >>unexpectedly, no callback at all invoked: neither 'visit_load', nor
>> >>'visit_store' (nor 'visit_address', obviously).
>> >
>> > The variables involved are registers. You only get called on memory 
>> > operands.
>>
>> How would I have told that from the 'walk_stmt_load_store_addr_ops'
>> function description?  (How to improve that one "to reflect relatity"?)
>>
>> But 'zzz' surely is the same in 'zzz = 1' vs. 'zzz = r + r2' -- for the
>> former I *do* see the 'visit_store' callback invoked, for the latter I
>> don't?
>
> The walk_gimple functions are intended to be used on the SSA form of
> gimple (i.e. the one that it is in most of the time).

Yes, "most of the time", but actually not in my case: I'm doing stuff
right after gimplification (before OMP lowering)...  So that's the
"detail" I was missing -- sorry for not mentioning that right away.  :-|

As 'walk_gimple_[...]' are used during gimplification, OMP lowering,
supposedly they're fine to use in non-SSA form -- but evidently some of
the helper functions are not.  (Might there be a way to add
'gcc_checking_assert (gimple_in_ssa_p (cfun))' or similar for that?
Putting that into 'walk_stmt_load_store_addr_ops' triggers a lot, as
called from 'gcc/cgraphbuild.c:cgraph_node::record_stmt_references', for
example.)

> And in that it's
> not the case that 'zzz = 1' and 'zzz = r + r2' are similar.  The former
> can have memory as the lhs (that includes static variables, or indirection
> through pointers), the latter can not.  The lhs of a binary statement is
> always an SSA name.  A write to an SSA name is not a store, which is why
> it's not walked for walk_stmt_load_store_addr_ops.
>
> Maybe it helps to look at simple C examples: [...]

I see, many thanks for reminding me about these items!

> If you want
> to capture writes into SSA names as well ([...])
> you need the per-operand callback indeed.

What I actually need is loads from/uses of actual variables (and I didn't
see 'walk_stmt_load_store_addr_ops' give me these).

> But that depends on
> what you actually want to do.

This is a prototype/"initial hack" for a very simple implementation of
 "Avoid unnecessary data transfer out of OMP
construct".  (... to be completed and posted later.)  So simple that it
will easily fail (gracefully, of course), but yet is effective for a lot
of real-world code:

subroutine [...]
[...]
real([...])   xx!temporary variable, for distance calculation
[...]
!$acc kernels pcopyin(x, zii) reduction(+:eva) ! implicit 'copy(xx)' for 
scalar used inside region; established during gimplification
  do 100 i=0,n-1
evx=0.0d0
do 90 j=0,n-1
  xx=abs(x(1,i)-x(1,j))
[...]
!$acc end kernels
[...]
['xx' never used here]
end subroutine [...]

Inside 'kernels', we'd like to automatically parallelize loops (we've
basically got that working; analysis by Graphite etc.), but the problem
is that given "implicit 'copy(xx)' for scalar used inside region", when
Graphite later looks at the outlined 'kernels' region's function, it must
assume that 'xx' is still live after the OpenACC 'kernels' construct --
and thus cannot treat it as a thread-private temporary, cannot
parallelize.

Now, walking each function backwards (!), I'm taking note of any
variables' uses, and if I reach an 'kernels' construct, but have not seen
a use of 'xx', I may then optimize 'copy(xx)' -> 'firstprivate(xx)',
enabling Graphite to do its thing.  (Other such optimizations may be
added later.)  (This is inspired by Jakub's commit
1a80d6b87d81c3f336ab199a901cf80ae349c335 "re PR tree-optimization/68128
(A huge regression in Parboil v2.5 OpenMP CUTCP test (2.5 times lower
performance))".)

I've now got a simple 'callback_op', which for '!is_lhs' looks at
'get_base_address ([op])', and if that 'var' is contained in the set of
current candidates (initialized per containg 'bind's, which we enter
first, even if walking a 'gimple_seq' backwards), removes that 'var' as a
candidate for such optimization.  (Plus some "details", of couse.)  This
seems to work fine, as far as I can tell.  :-)


Of course, the eventual IPA-based solution (see PR90591, PR94