GCC aliasing extension for C

[Florian Weimer]

I know that GCC implements a C extension (which is more or less required 
by POSIX) which allows you to define a buffer


  char buf[1024];

and then allocate objects from that (assuming that the buffer is 
sufficiently large and the pointers to subobjects are suitably aligned). 
 In short, it is possible to change the dynamic type of an object even 
if it was defined with a type.


The object is first written under the new type, and only accessed under 
the new type.  It is also possible that the buffer is reused and 
different objects with different types at the same offsets are carved 
out from it.


Is this a property of the char type, or would other types work as well, 
for example, double or long double?


Thanks,
Florian

Re: Infering that the condition of a for loop is initially true?

[Niels Möller]

Marc Glisse  writes:

> assert is not what you want, since it completely disappears with
> -DNDEBUG. clang has __builtin_assume, with gcc you want a test and
> __builtin_unreachable. Replacing your assert with
> if(n==0)__builtin_unreachable();

To me, it would make sense to have assert expand to something like that,
when runtime checks are disabled with -NDEBUG.

After all, is a program fails an assertion, it can not be expected to
produce any meaningful results, even with run-time checks disabled using
-DNDEBUG. And it's somewhat counter-intutive, if -DNDEBUG produces a
binary in which the code downstream from assert statements are slightly
less well optimized.

I imagine the C standard might specify that assert "completely
disappears" if -DNDEBUG is defined. But maybe there's room for a
-fstrict-assert optimization flag, to let assert always guide
optimization?

Regards,
/Niels

-- 
Niels Möller. PGP-encrypted email is preferred. Keyid 368C6677.
Internet email is subject to wholesale government surveillance.

Re: Infering that the condition of a for loop is initially true?

[Andreas Schwab]

On Sep 18 2017, ni...@lysator.liu.se (Niels Möller) wrote:

> I imagine the C standard might specify that assert "completely
> disappears" if -DNDEBUG is defined. But maybe there's room for a
> -fstrict-assert optimization flag, to let assert always guide
> optimization?

The problem is that assert is not allowed to evaluate the expression
with -DNDEBUG, and side effects must not be carried out.

Andreas.

-- 
Andreas Schwab, SUSE Labs, sch...@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

Re: Infering that the condition of a for loop is initially true?

[Niels Möller]

Jeff Law  writes:

>> 1. Does gcc attempt to do this optimization? 
> Yes.  It happens as a side effect of jump threading and there are also
> dedicated passes to rotate the loop.
>
>> 
>> 2. If it does, how often does it succeed on loops in real programs?
> Often.  The net benefit is actually small though and sometimes this kind
> of loop rotation can impede vectorization.

Thanks for the info.

>> 3. Can I help the compiler to do that inference?
> In general, I'd advise against it.  You end up with ugly code which
> works with specific versions of the compiler, but which needs regular
> tweaking as the internal implementations of various optimizers change
> over time.

What would make sense to me is some annotation of valid range of
variables, if it can be done in a way which is friendly to both compiler
and humans.

> I'd have to have a self-contained example to dig into what's really
> going on, but my suspicion is either overflow or fairly weak range data
> and simplification due to the symbolic ranges.

Self-contained example below (the #define of GMP_NUMB_BITS must be
manually changed if tested on some architecture where long isn't 64
bits).

Actually, I see now that the mn variable is read from a struct field of
type unsigned short. So as long as size_t (or unsigned long) is larger
than unsigned short, the expression 2*mn can't overflow, and a compiler
tracking possible range of mn as the range of unsigned short should be
able to infer that the loop condition of the second loop is initially
true.

To make the same inference for the first loop (which doesn't matter for
validity of the hi variable), the compiler would also need to be told
that bn > 0.

Regards,
/Niels

typedef unsigned long mp_size_t;
typedef unsigned long mp_limb_t;
typedef mp_limb_t *mp_ptr;
typedef const mp_limb_t *mp_srcptr;
/* Must match szie of mp_limb_t */
#define GMP_NUMB_BITS 64

#define assert(x) do {} while(0)

mp_limb_t mpn_addmul_1 (mp_ptr, mp_srcptr, mp_size_t, mp_limb_t);
mp_limb_t mpn_add_n (mp_ptr, mp_srcptr, mp_srcptr, mp_size_t);
mp_limb_t
sec_add_1 (mp_limb_t *rp, mp_limb_t *ap, mp_size_t n, mp_limb_t b);
#define cnd_add_n(cnd, rp, ap, n) mpn_addmul_1 ((rp), (ap), (n), (cnd) != 0)

struct ecc_modulo
{
  unsigned short bit_size;
  unsigned short size;
  unsigned short B_size;

  const mp_limb_t *m;
  /* B^size mod m. Expected to have at least 32 leading zeros
 (equality for secp_256r1). */
  const mp_limb_t *B;
  /* 2^{bit_size} - p, same value as above, but shifted. */
  const mp_limb_t *B_shifted;
};

/* Computes r mod m, input 2*m->size, output m->size. */
void
ecc_mod (const struct ecc_modulo *m, mp_limb_t *rp)
{
  mp_limb_t hi;
  mp_size_t mn = m->size;
  mp_size_t bn = m->B_size;
  mp_size_t sn = mn - bn;
  mp_size_t rn = 2*mn;
  mp_size_t i;
  unsigned shift;

  assert (bn < mn);

  /* FIXME: Could use mpn_addmul_2. */
  /* Eliminate sn limbs at a time */
  if (m->B[bn-1] < ((mp_limb_t) 1 << (GMP_NUMB_BITS - 1)))
{
  /* Multiply sn + 1 limbs at a time, so we get a mn+1 limb
 product. Then we can absorb the carry in the high limb */
  while (rn > 2 * mn - bn)
{
  rn -= sn;

  for (i = 0; i <= sn; i++)
rp[rn+i-1] = mpn_addmul_1 (rp + rn - mn - 1 + i, m->B, bn, 
rp[rn+i-1]);
  rp[rn-1] = rp[rn+sn-1]
+ mpn_add_n (rp + rn - sn - 1, rp + rn - sn - 1, rp + rn - 1, sn);
}
  goto final_limbs;
}
  else
{
  /* The loop below always runs at least once. But the analyzer
 doesn't realize that, and complains about hi being used later
 on without a well defined value. */
#ifdef __clang_analyzer__
  hi = 0;
#endif
  while (rn >= 2 * mn - bn)
{
  rn -= sn;

  for (i = 0; i < sn; i++)
rp[rn+i] = mpn_addmul_1 (rp + rn - mn + i, m->B, bn, rp[rn+i]);
 
  hi = mpn_add_n (rp + rn - sn, rp + rn - sn, rp + rn, sn);
  hi = cnd_add_n (hi, rp + rn - mn, m->B, mn);
  assert (hi == 0);
}
}

  if (rn > mn)
{
final_limbs:
  sn = rn - mn;
  
  for (i = 0; i < sn; i++)
rp[mn+i] = mpn_addmul_1 (rp + i, m->B, bn, rp[mn+i]);

  hi = mpn_add_n (rp + bn, rp + bn, rp + mn, sn);
  hi = sec_add_1 (rp + bn + sn, rp + bn + sn, mn - bn - sn, hi);
}

  shift = m->size * GMP_NUMB_BITS - m->bit_size;
  if (shift > 0)
{
  /* Combine hi with top bits, add in */
  hi = (hi << shift) | (rp[mn-1] >> (GMP_NUMB_BITS - shift));
  rp[mn-1] = (rp[mn-1] & (((mp_limb_t) 1 << (GMP_NUMB_BITS - shift)) - 1))
+ mpn_addmul_1 (rp, m->B_shifted, mn-1, hi);
}
  else
{
  hi = cnd_add_n (hi, rp, m->B_shifted, mn);
  assert (hi == 0);
}
}



-- 
Niels Möller. PGP-encrypted email is preferred. Keyid 368C6677.
Internet email is subject to wholesale government surveillance.

Re: Infering that the condition of a for loop is initially true?

[Niels Möller]

Andreas Schwab  writes:

> The problem is that assert is not allowed to evaluate the expression
> with -DNDEBUG, and side effects must not be carried out.

I'm suggesting that with -DNDEBUG, assert(x) should let the compiler
assume that x is true, but without producing any code to evaluate it at
runtime.

E.g, given

  void foo (size_t n)
  {
size_t i;
assert (n < 17);
for (i = n; i < 17; i++)
  { ... }
  }

the compiler could infer that for the loop body is executed at least
once for any valid input to the function. (Where assert is used to
declare what valid inputs are like).

Then I guess one can't implement that as simply as

#define assert(x) do {if (!(x)) __builtin_unreachable() } while(0)

if that would require the compiler to generate code to evaluate x at
runtime (I'm not familiar with how __builtin_unreachable works). So one
might need something like __builtin_assume which never produces any code
to evaluate its argument at runtime.

Regarsd,
/Niels

-- 
Niels Möller. PGP-encrypted email is preferred. Keyid 368C6677.
Internet email is subject to wholesale government surveillance.

Re: Infering that the condition of a for loop is initially true?

[Jeffrey Walton]

> I'm suggesting that with -DNDEBUG, assert(x) should let the compiler
> assume that x is true, but without producing any code to evaluate it at
> runtime.
>
> E.g, given
>
>   void foo (size_t n)
>   {
> size_t i;
> assert (n < 17);
> for (i = n; i < 17; i++)
>   { ... }
>   }

I think what people usually do (for the folks who do it) is provide a
verify() macro that always survives. In debug builds verify() also
calls assert() to snap the debugger.

Jeff

GCC extension for atomic access to members

[Florian Weimer]

I would like to see the GCC project to document that if the address of a 
member is taken, this does not constitute an access to the object as a 
whole.


That is, in the following code:

#include 

struct S {
  _Atomic int a;
  int b;
};

int
load_a (struct S *p)
{
  return atomic_load_explicit (&p->a, memory_order_relaxed);
}

int
store_b (struct S *p, int b)
{
  p->b = b;
}

If one thread calls load_a and another thread calls store_b on the same 
struct S *, no data race happens.


This is an extension over the C standard because of the way “->” is 
defined.  C requires that E1->E2 it is evaluated as (*(E1))->E2, and *E1 
is defined as an access to the entire struct, so there is a data race in 
load_a with the assignment in store_b.


This is somewhat complicated to fix, wording-wise, because for the 
purpose of aliasing analysis, we need the whole-struct access to *E1, 
otherwise there is nothing that asserts the dynamic type of this memory 
location.  But without such an assertion, the alias analysis GCC 
currently performs would be wrong.


A similar extension is needed for access to embedded synchronization 
types (such as mutexes), but it is even harder to express properly.


Of course, there is a workaround to make this issue go away, like this:

#define member_type(type, member) __typeof__ (((type) {}).member)
#define member_address(this, member) \
  (member_type (__typeof__ (*(this)), member) *) \
  (((char *) (this) + offsetof (__typeof__ (*(this)), member)))

int
load_a_ (struct S *p)
{
  return atomic_load_explicit (member_address (p, a), 
memory_order_relaxed);

}

int
store_b_ (struct S *p, int b)
{
  *member_address (p, b) = b;
}

But it seems to be silly to write code this way.  (We actually use these 
macros in libio, where we have to implement C++ class inheritance in C.)


For C++, we can probably fix this in the standard in some way, so that 
no extension is required.


Thanks,
Florian

Re: GCC aliasing extension for C

[Andrew Haley]

On 18/09/17 10:48, Florian Weimer wrote:
> Is this a property of the char type, or would other types work as well, 
> for example, double or long double?

It has to be a character type, I believe.

-- 
Andrew Haley
Java Platform Lead Engineer
Red Hat UK Ltd. 
EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671

Re: Infering that the condition of a for loop is initially true?

[Joseph Myers]

On Mon, 18 Sep 2017, Niels Möller wrote:

> Andreas Schwab  writes:
> 
> > The problem is that assert is not allowed to evaluate the expression
> > with -DNDEBUG, and side effects must not be carried out.
> 
> I'm suggesting that with -DNDEBUG, assert(x) should let the compiler
> assume that x is true, but without producing any code to evaluate it at
> runtime.

There's no requirement that x is even a valid expression with -DNDEBUG.  
Consider code that does

  int x;
#ifndef NDEBUG
  int other_variable_used_in_assertion = something ();
#endif
  /* ... */
  assert (other_variable_used_in_assertion == x);

where it is completely valid in ISO C not to have the NDEBUG conditional 
around the assert call, because in the NDEBUG case the macro argument 
won't be expanded.

-- 
Joseph S. Myers
jos...@codesourcery.com

Re: GCC aliasing extension for C

[Richard Biener]

On September 18, 2017 4:12:07 PM GMT+02:00, Andrew Haley  
wrote:
>On 18/09/17 10:48, Florian Weimer wrote:
>> Is this a property of the char type, or would other types work as
>well, 
>> for example, double or long double?
>
>It has to be a character type, I believe.

It can be any type. All stores change the dynamic type of the memory accessed. 

Richard. 

Re: Infering that the condition of a for loop is initially true?

[Niels Möller]

Joseph Myers  writes:

> On Mon, 18 Sep 2017, Niels Möller wrote:

>> I'm suggesting that with -DNDEBUG, assert(x) should let the compiler
>> assume that x is true, but without producing any code to evaluate it at
>> runtime.
>
> There's no requirement that x is even a valid expression with -DNDEBUG.  
> Consider code that does
>
>   int x;
> #ifndef NDEBUG
>   int other_variable_used_in_assertion = something ();
> #endif
>   /* ... */
>   assert (other_variable_used_in_assertion == x);

Ouch, didn't think about that case. And I'd expect there's a lot of real
code like that out there.

That makes extending the standard assert facility more difficult.

Still, reusing the familiar name "assert" in the source code would be
nice, but the behavior I ask for would need to be triggered by something
other than -DNDEBUG, a gcc-specific define or command line option. And I
guess we can't hijack something like -DNDEBUG=2, even though I'd expect
that to be pretty close to working in practice.

Regards,
/Niels

-- 
Niels Möller. PGP-encrypted email is preferred. Keyid 368C6677.
Internet email is subject to wholesale government surveillance.

Re: [RFC] type promotion pass

[Steve Ellcey]

On Fri, 2017-09-15 at 12:22 +, Wilco Dijkstra wrote:

Wilco or Prathamesh,

I could not apply this patch (cleanly) to ToT.  match.pd did not apply,
I think I fixed that.  The cfgexpand.c patch applied but will not
build.  I get this error:

../../../src/gcc/gcc/cfgexpand.c: In function ‘rtx_def*
expand_debug_expr(tree)’:
../../../src/gcc/gcc/cfgexpand.c:5130:18: error: cannot convert
‘opt_machine_mode {aka opt_mode}’ to ‘machine_mode’ in
assignment
   inner_mode = mode_for_size (INTVAL (op1), MODE_INT, 0);


I can't quite figure out what change needs to be made to this line to
make it compile.  I do see that mode_for_size has been changed.
I tried using int_mode_for_size but that doesn't work and I tried
using '.require ()' but that didn't work either.

inner_mode = int_mode_for_size (INTVAL (op1), 0);  /* This did not work.  */
inner_mode = mode_for_size (INTVAL (op1), MODE_INT, 0).require (); /* This did 
not work */

Steve Ellcey
sell...@cavium.com

Re: [RFC] type promotion pass

[Prathamesh Kulkarni]

On 18 September 2017 at 23:12, Steve Ellcey  wrote:
> On Fri, 2017-09-15 at 12:22 +, Wilco Dijkstra wrote:
>
> Wilco or Prathamesh,
>
> I could not apply this patch (cleanly) to ToT.  match.pd did not apply,
> I think I fixed that.  The cfgexpand.c patch applied but will not
> build.  I get this error:
Hi Steve,
The patch is currently based on r249469. I will rebase it on ToT and
look into the build failure.
Thanks for pointing it out.

Regards,
Prathamesh
>
> ../../../src/gcc/gcc/cfgexpand.c: In function ‘rtx_def*
> expand_debug_expr(tree)’:
> ../../../src/gcc/gcc/cfgexpand.c:5130:18: error: cannot convert
> ‘opt_machine_mode {aka opt_mode}’ to ‘machine_mode’ in
> assignment
>inner_mode = mode_for_size (INTVAL (op1), MODE_INT, 0);
>
>
> I can't quite figure out what change needs to be made to this line to
> make it compile.  I do see that mode_for_size has been changed.
> I tried using int_mode_for_size but that doesn't work and I tried
> using '.require ()' but that didn't work either.
>
> inner_mode = int_mode_for_size (INTVAL (op1), 0);  /* This did not work.  */
> inner_mode = mode_for_size (INTVAL (op1), MODE_INT, 0).require (); /* This 
> did not work */
>
> Steve Ellcey
> sell...@cavium.com
>
>

PR68425 warn message enhancement

[Prasad Ghangal]

Hi,

This was discussed a few time ago in this email thread :
https://gcc.gnu.org/ml/gcc/2016-02/msg00235.html

As per the discussion, the attached patch produces warning message for
excess elements in array initialization along with number of expected
and extra elements and underlined extra elements ranges.

e.g for following testcases:

int foo()
{
  int a[] = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10};
  int b[0] = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10};
  int c[5] = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10};
  int d[5][2] = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10};
  int e[5][0] = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10};
  int f[5][2][3] = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 1, 2, 3, 4, 5, 6,
7, 8, 9, 10, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 1, 2, 3, 4, 5, 6, 7, 8, 9,
10};
  int h[3][2][3][2] = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 1, 2, 3, 4, 5,
6, 7, 8, 9, 10, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 1, 2, 3, 4, 5, 6, 7};
  return 0;
}


It produces (gmail may mess up with spaces):

./gcc/cc1 ../tests/test.c
 foo
../tests/test.c: In function ‘foo’:
../tests/test.c:4:15: warning: excess elements in array initializer
(10 elements, 0 expected)
   int b[0] = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10};
   ^~~
../tests/test.c:4:15: note: (near initialization for ‘b’)
../tests/test.c:5:30: warning: excess elements in array initializer
(10 elements, 5 expected)
   int c[5] = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10};
   ^~~
../tests/test.c:5:30: note: (near initialization for ‘c’)
../tests/test.c:6:49: warning: excess elements in array initializer
(20 elements, 10 expected)
   int d[5][2] = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10};

^~~
../tests/test.c:6:49: note: (near initialization for ‘d[5]’)
../tests/test.c:7:18: warning: excess elements in array initializer
(20 elements, 0 expected)
   int e[5][0] = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10};
   ^~~
../tests/test.c:7:18: note: (near initialization for ‘e[5]’)
../tests/test.c:8:114: warning: excess elements in array initializer
(40 elements, 30 expected)
   int f[5][2][3] = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 1, 2, 3, 4, 5, 6,
7, 8, 9, 10, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10};
^~~
../tests/test.c:8:114: note: (near initialization for ‘f[5][1]’)
../tests/test.c:9:135: warning: excess elements in array initializer
(37 elements, 36 expected)
   int h[3][2][3][2] = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 1, 2, 3, 4, 5,
6, 7, 8, 9, 10, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 1, 2, 3, 4, 5, 6, 7};

 ^
../tests/test.c:9:135: note: (near initialization for ‘h[3][0][0]’)


Any thoughts?


Thanks,
Prasad


PR68425.patch
Description: Binary data

Re: [RFC] type promotion pass

[Steve Ellcey]

On Mon, 2017-09-18 at 23:29 +0530, Prathamesh Kulkarni wrote:
> 
> Hi Steve,
> The patch is currently based on r249469. I will rebase it on ToT and
> look into the build failure.
> Thanks for pointing it out.
> 
> Regards,
> Prathamesh

OK, I applied it to that version successfully.  The thing I wanted to
check was to see if this helped with PR target/77729.  It does not,
so I think even with this patch we would need my patch to address the
issue of having GCC recognize that ldrb/ldhb zero out the top of a
register and thus we do not need to mask it out later.

https://gcc.gnu.org/ml/gcc-patches/2017-09/msg00929.html

Steve Ellcey
sell...@cavium.com

Re: PR68425 warn message enhancement

[Martin Sebor]

On 09/18/2017 01:06 PM, Prasad Ghangal wrote:

Hi,

This was discussed a few time ago in this email thread :
https://gcc.gnu.org/ml/gcc/2016-02/msg00235.html

As per the discussion, the attached patch produces warning message for
excess elements in array initialization along with number of expected
and extra elements and underlined extra elements ranges.


I like the additional detail and the underlining of all the excess
elements when the whole initializer list is on the same line.  I'm
not sure it's a good idea to underline all the excess elements when
they are on separate lines.  That could make the warning really long.

For some additional improvements I would suggest either eliminating
the note (it serves no useful purpose anymore) or replacing it with
the element count(s).  The message printing the element counter
should also be adjusted to be correct for zero-length arrays as in:

  int a[0] = { 1 };

Alternatively, change the message to say just:

  warning: X excess elements in an initializer for 'int a[N]'

and when the line the caret is on is not the same as the object
being initialized, print a note pointing to the declaration as
is done by some other warnings:

  note: array 'a' declared here

As an additional improvement, string initializers with excessive
elements would benefit from the same underlining/element counts.

The C++ front end will need similar changes.

Finally, it would be nice to also be able to control the warning
(i.e., provide an option to turn it on and off).

Martin

PS Please send patches to gcc-patches.



e.g for following testcases:

int foo()
{
  int a[] = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10};
  int b[0] = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10};
  int c[5] = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10};
  int d[5][2] = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10};
  int e[5][0] = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10};
  int f[5][2][3] = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 1, 2, 3, 4, 5, 6,
7, 8, 9, 10, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 1, 2, 3, 4, 5, 6, 7, 8, 9,
10};
  int h[3][2][3][2] = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 1, 2, 3, 4, 5,
6, 7, 8, 9, 10, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 1, 2, 3, 4, 5, 6, 7};
  return 0;
}


It produces (gmail may mess up with spaces):

./gcc/cc1 ../tests/test.c
 foo
../tests/test.c: In function ‘foo’:
../tests/test.c:4:15: warning: excess elements in array initializer
(10 elements, 0 expected)
   int b[0] = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10};
   ^~~
../tests/test.c:4:15: note: (near initialization for ‘b’)
../tests/test.c:5:30: warning: excess elements in array initializer
(10 elements, 5 expected)
   int c[5] = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10};
   ^~~
../tests/test.c:5:30: note: (near initialization for ‘c’)
../tests/test.c:6:49: warning: excess elements in array initializer
(20 elements, 10 expected)
   int d[5][2] = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10};

^~~
../tests/test.c:6:49: note: (near initialization for ‘d[5]’)
../tests/test.c:7:18: warning: excess elements in array initializer
(20 elements, 0 expected)
   int e[5][0] = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10};
   ^~~
../tests/test.c:7:18: note: (near initialization for ‘e[5]’)
../tests/test.c:8:114: warning: excess elements in array initializer
(40 elements, 30 expected)
   int f[5][2][3] = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 1, 2, 3, 4, 5, 6,
7, 8, 9, 10, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10};
^~~
../tests/test.c:8:114: note: (near initialization for ‘f[5][1]’)
../tests/test.c:9:135: warning: excess elements in array initializer
(37 elements, 36 expected)
   int h[3][2][3][2] = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 1, 2, 3, 4, 5,
6, 7, 8, 9, 10, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 1, 2, 3, 4, 5, 6, 7};

 ^
../tests/test.c:9:135: note: (near initialization for ‘h[3][0][0]’)


Any thoughts?


Thanks,
Prasad

Re: GCC extension for atomic access to members

[Torvald Riegel]

On Mon, 2017-09-18 at 14:19 +0200, Florian Weimer wrote:
> I would like to see the GCC project to document that if the address of a 
> member is taken, this does not constitute an access to the object as a 
> whole.
> 
> That is, in the following code:
> 
> #include 
> 
> struct S {
>_Atomic int a;
>int b;
> };
> 
> int
> load_a (struct S *p)
> {
>return atomic_load_explicit (&p->a, memory_order_relaxed);
> }
> 
> int
> store_b (struct S *p, int b)
> {
>p->b = b;
> }
> 
> If one thread calls load_a and another thread calls store_b on the same 
> struct S *, no data race happens.

There is no data race in this example; a and b are separate objects as
far as the memory model is concerned.  That's my understanding and I
believe also the understanding in the C++ committee.

> This is an extension over the C standard because of the way “->” is 
> defined.  C requires that E1->E2 it is evaluated as (*(E1))->E2, and *E1 
> is defined as an access to the entire struct, so there is a data race in 
> load_a with the assignment in store_b.

If ISO C really says that this is potentially a data race, then that's a
bug in the standard I'd say.  I see no need to have an extension for
this (module there potentially being a real bug in the standard).

Re: Redundant sign-extension instructions on RISC-V

[Michael Clark]

Hi,

I’ve spun an alternative version of RISC-V GCC which preserves RV64 ABI sign 
extended canonicalisation for passed values but keeps values in SImode until 
return.

https://github.com/michaeljclark/riscv-gcc/commits/riscv-gcc-7-mc

These are the changes:

- #undef PROMOTE_MODE
- #define TARGET_PROMOTE_FUNCTION_MODE
- Implement riscv_function_promote to promote arguments and return 
values
- Add SImode logical op patterns to riscv.md

The RTL for the sign extension regression test cases now contain all SImode 
ops, however REE is still not able to remove sign_extend after and/ior/xor/not 
with SImode inputs that are in canonical form. I’m still seeing shifts not 
being coalesced as this likely happens in combine, and the redundant sign 
extensions are not eliminated until much later in REE with this particular 
configuration.

Observations.

- Aarch64 leaves upper bits undefined instead of canonicalising them 
and PROMOTE_MODE promotes QImode and HImode to SImode.
- RISC-V defines explicit sign_extend + op pattern matches in its 
metadata to collapse redundant sign extends
- Explicit _extend metadata patterns appear to be required for REE to 
eliminate redundant sign_extend.
- I removed these patterns and then all functions returning int 
had: sext.w a0, a0; ret
- Explicit _extend metadata patterns span only two ops
- e.g. (sign_extend:DI (plus:SI)), (sign_extend:DI (minus:SI)), 
(sign_extend:DI (any_shift:SI)), etc
- REE may not be able to cross enough transitive dependencies to handle 
sign_extend and/ior/xor/not ops with canonical inputs
- e.g. (sign_extend:DI (logical_op:SI (plus_minus_shift:SI) 
(plus_minus_shift:SI)))
- e.g. (sign_extend:DI (logical_op:SI (logical_op:SI 
(logical_op:SI (plus_minus_shift:SI) (plus_minus_shift:SI)
- metadata pattern matching approach is untenable where there 
is a series of logical ops on canonical inputs. e.g. the bswap test case
- Shift coalescing is done before REE in combine so this may be why we 
are seeing missed optimisations.
- there may be more optimisations that are missed due to the 
sign_extend promotions changing patterns
- Keeping values in SImode until return doesn’t appear to help much 
however it may change how the problem is solved.

Actions

- Backport Prathamesh’s type promotion patch to riscv-gcc. It may suit 
the eager definition of PROMOTE_MODE
- Benchmark Alternative ABI that leaves upper bits undefined, out of 
curiosities sake…

I’ve been reasoning about the alternate ABI. I can see it has the cost of 
widening narrower return values when they are promoted to wider types, but the 
general case of adding integers to pointers would not be affected when it is 
inline. Deferring canonicalisation when there are several routines that return 
ints and work on ints as inputs would seem to indicate generation of less 
operations. I’d really like to see SPECint run on the alternate ABI. However 
the current ABI could be improved if the compiler is better able to reason 
about cases where the sign_extend operations don’t need to be emitted, so it 
would not be a fair test against the current version of the port.

Michael.

> On 30 Aug 2017, at 12:36 PM, Michael Clark  wrote:
> 
> Dear GCC folk,
> 
> 
> # Issue Background
> 
> We’re investigating an issue with redundant sign-extension instructions being 
> emitted with the riscv backend. Firstly I would like to state that riscv is 
> possibly a unique backend with respect to its canonical sign-extended 
> register form due to the following:
> 
> - POINTERS_EXTEND_UNSIGNED is false, and the canonical form after 32-bit 
> operations on RV64 is sign-extended not zero-extended.
> 
> - PROMOTE_MODE is defined on RV64 such that SI mode values are promoted to 
> signed DI mode values holding SI mode subregs
> 
> - RISC-V does not have register aliases for these different modes, rather 
> different instructions are selected to operate on different register widths.
> 
> - The 32-bit instructions sign-extend results. e.g. all shifts, add, sub, etc.
> 
> - Some instructions such as logical ops only have full word width variants 
> (AND, OR, XOR) but these instructions maintain canonical form as there is no 
> horizontal movement of bits.
> 
> 
> # Issue Details
> 
> During porting from GCC 6.1 to GCC 7.1, the RISC-V port was changed to define 
> TRULY_NOOP_TRUNCATION to 1 and PROMOTE_MODE was set up to promote values to 
> wider modes. This was done to ensure ABI correctness so that registers 
> holding narrower modes always contain canonical sign extended values.
> 
> The result of this is that sign_extend operations are inserted but later 
> eliminated in ree/simplyfy_rtx. In testcase 3 the sign_extend is correctly 
> eliminated, and indeed most 32-bit instructions are handled correctly. This 
> is what the pa

Re: [RFC] type promotion pass

[Kugan Vivekanandarajah]

Hi Steve,

On 19 September 2017 at 05:45, Steve Ellcey  wrote:
> On Mon, 2017-09-18 at 23:29 +0530, Prathamesh Kulkarni wrote:
>>
>> Hi Steve,
>> The patch is currently based on r249469. I will rebase it on ToT and
>> look into the build failure.
>> Thanks for pointing it out.
>>
>> Regards,
>> Prathamesh
>
> OK, I applied it to that version successfully.  The thing I wanted to
> check was to see if this helped with PR target/77729.  It does not,
> so I think even with this patch we would need my patch to address the
> issue of having GCC recognize that ldrb/ldhb zero out the top of a
> register and thus we do not need to mask it out later.
>
> https://gcc.gnu.org/ml/gcc-patches/2017-09/msg00929.html

I tried the testases you have in the patch with type promotion. Looks
like forwprop is reversing the promotion there. I haven't looked in
detail yet but -fno-tree-forwprop seems to remove 6 "and" from the
test case. I have a slightly different version to what Prathamseh has
posted and hope that there isn't any difference here.

Thanks,
Kugan

Re: GCC extension for atomic access to members

[Florian Weimer]

On 09/18/2017 10:07 PM, Torvald Riegel wrote:

On Mon, 2017-09-18 at 14:19 +0200, Florian Weimer wrote:

I would like to see the GCC project to document that if the address of a
member is taken, this does not constitute an access to the object as a
whole.

That is, in the following code:

#include 

struct S {
_Atomic int a;
int b;
};

int
load_a (struct S *p)
{
return atomic_load_explicit (&p->a, memory_order_relaxed);
}

int
store_b (struct S *p, int b)
{
p->b = b;
}

If one thread calls load_a and another thread calls store_b on the same
struct S *, no data race happens.


There is no data race in this example; a and b are separate objects as
far as the memory model is concerned.  That's my understanding and I
believe also the understanding in the C++ committee.


I'm not sure what the C++ standard says in this matter.

C++ may indeed have addressed this and related issues (e.g., that a 
memory location which is passed by reference to a function is not 
actually read in the caller).



This is an extension over the C standard because of the way “->” is
defined.  C requires that E1->E2 it is evaluated as (*(E1))->E2, and *E1
is defined as an access to the entire struct, so there is a data race in
load_a with the assignment in store_b.


If ISO C really says that this is potentially a data race, then that's a
bug in the standard I'd say.  I see no need to have an extension for
this (module there potentially being a real bug in the standard).


GCC relies on the implicit access to the full struct object in this case:

struct T {
  int a;
  int b;
};

int
opt (struct T *p1, struct T *p2)
{
  p1->a = 1;
  p2->b = 2;
  return p1->a;
}

It optimizes the load of p1->a to 1.  This is not a valid transformation 
if there is no access to the entire struct object because it's the only 
thing in this code which asserts the dynamic type of the memory 
allocation is that of T, so that the members a and b do not alias.


This is why I think this issue is difficult to fix.

Thanks,
Florian

Re: PR68425 warn message enhancement

[Prasad Ghangal]

On 19 September 2017 at 01:35, Martin Sebor  wrote:
> On 09/18/2017 01:06 PM, Prasad Ghangal wrote:
>>
>> Hi,
>>
>> This was discussed a few time ago in this email thread :
>> https://gcc.gnu.org/ml/gcc/2016-02/msg00235.html
>>
>> As per the discussion, the attached patch produces warning message for
>> excess elements in array initialization along with number of expected
>> and extra elements and underlined extra elements ranges.
>
>
> I like the additional detail and the underlining of all the excess
> elements when the whole initializer list is on the same line.  I'm
> not sure it's a good idea to underline all the excess elements when
> they are on separate lines.  That could make the warning really long.
>
> For some additional improvements I would suggest either eliminating
> the note (it serves no useful purpose anymore) or replacing it with
> the element count(s).  The message printing the element counter
> should also be adjusted to be correct for zero-length arrays as in:
>
>   int a[0] = { 1 };
>
> Alternatively, change the message to say just:
>
>   warning: X excess elements in an initializer for 'int a[N]'
>
> and when the line the caret is on is not the same as the object
> being initialized, print a note pointing to the declaration as
> is done by some other warnings:
>
>   note: array 'a' declared here
>
> As an additional improvement, string initializers with excessive
> elements would benefit from the same underlining/element counts.
>
> The C++ front end will need similar changes.
>
> Finally, it would be nice to also be able to control the warning
> (i.e., provide an option to turn it on and off).
>
Currently gcc raises warning for excess elements in an initializer by default.
Should we introduce new flag/option (like
Wextra-elements-initializers) or use any existing one?

> Martin
>
> PS Please send patches to gcc-patches.
>
>
>>
>> e.g for following testcases:
>>
>> int foo()
>> {
>>   int a[] = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10};
>>   int b[0] = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10};
>>   int c[5] = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10};
>>   int d[5][2] = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 1, 2, 3, 4, 5, 6, 7, 8, 9,
>> 10};
>>   int e[5][0] = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 1, 2, 3, 4, 5, 6, 7, 8, 9,
>> 10};
>>   int f[5][2][3] = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 1, 2, 3, 4, 5, 6,
>> 7, 8, 9, 10, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 1, 2, 3, 4, 5, 6, 7, 8, 9,
>> 10};
>>   int h[3][2][3][2] = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 1, 2, 3, 4, 5,
>> 6, 7, 8, 9, 10, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 1, 2, 3, 4, 5, 6, 7};
>>   return 0;
>> }
>>
>>
>> It produces (gmail may mess up with spaces):
>>
>> ./gcc/cc1 ../tests/test.c
>>  foo
>> ../tests/test.c: In function ‘foo’:
>> ../tests/test.c:4:15: warning: excess elements in array initializer
>> (10 elements, 0 expected)
>>int b[0] = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10};
>>^~~
>> ../tests/test.c:4:15: note: (near initialization for ‘b’)
>> ../tests/test.c:5:30: warning: excess elements in array initializer
>> (10 elements, 5 expected)
>>int c[5] = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10};
>>^~~
>> ../tests/test.c:5:30: note: (near initialization for ‘c’)
>> ../tests/test.c:6:49: warning: excess elements in array initializer
>> (20 elements, 10 expected)
>>int d[5][2] = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 1, 2, 3, 4, 5, 6, 7, 8,
>> 9, 10};
>>
>> ^~~
>> ../tests/test.c:6:49: note: (near initialization for ‘d[5]’)
>> ../tests/test.c:7:18: warning: excess elements in array initializer
>> (20 elements, 0 expected)
>>int e[5][0] = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 1, 2, 3, 4, 5, 6, 7, 8,
>> 9, 10};
>>^~~
>> ../tests/test.c:7:18: note: (near initialization for ‘e[5]’)
>> ../tests/test.c:8:114: warning: excess elements in array initializer
>> (40 elements, 30 expected)
>>int f[5][2][3] = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 1, 2, 3, 4, 5, 6,
>> 7, 8, 9, 10, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 1, 2, 3, 4, 5, 6, 7, 8, 9,
>> 10};
>>
>> ^~~
>> ../tests/test.c:8:114: note: (near initialization for ‘f[5][1]’)
>> ../tests/test.c:9:135: warning: excess elements in array initializer
>> (37 elements, 36 expected)
>>int h[3][2][3][2] = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 1, 2, 3, 4, 5,
>> 6, 7, 8, 9, 10, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 1, 2, 3, 4, 5, 6, 7};
>>
>>  ^
>> ../tests/test.c:9:135: note: (near initialization for ‘h[3][0][0]’)
>>
>>
>> Any thoughts?
>>
>>
>> Thanks,
>> Prasad
>>
>