(In reply to Florian La Roche from comment #0)
> Created attachment 44444 [details]
> testcase
> 
> I can see this on x86_64 and aarch64. The first function is compiled with
> much
> bigger code. Seems the alignment to 8 bytes and thus this multiple of 8
> is forgotten in some optimization step.

Can you please explain what precisely is wrong by 'is compiled with much bigger code'?

The extra insns are introduced in the ivopts pass.

  <bb 5> [local count: 105119325]:
  ivtmp.7_7 = (unsigned long) &__bss_start1;
  _11 = (unsigned long) &__bss_end1;
  _12 = _11 + 18446744073709551615;
  _13 = (unsigned long) &__bss_start1;
  _14 = _12 - _13;
  _15 = _14 & 18446744073709551608;
  _16 = (unsigned long) &__bss_start1;
  _17 = _16 + 8;
  _18 = _15 + _17;

Hello Martin,

I assume the two functions clear_bss1() and clear_bss2() to work on
identical aligned data and produce similar assembler output.
Yet looking at the assembler output, the first function produces
many more assembler lines. "-Os" keeps the assembler lines also pretty small.

The first assembler listing should remove "18" to "2C" the last
listing should remove "13" to "22".



Here another output from gcc, where the additional pseudocode shows up
after optimizations. The lines with pseudo vars "_13" to "_20" should
not be produced at all.



;; Function clear_bss1 (clear_bss1, funcdef_no=0, decl_uid=3118, cgraph_uid=0, symbol_order=0)

Removing basic block 6
Removing basic block 7
Removing basic block 8
clear_bss1 ()
{
  unsigned long ivtmp.9;
  void * _11;
  unsigned long _12;
  unsigned long _13;
  unsigned long _16;
  unsigned long _17;
  unsigned long _18;
  unsigned long _19;
  unsigned long _20;

  <bb 2> [15.00%]:
  if (&__bss_start1 < &__bss_end1)
    goto <bb 3>; [85.00%]
  else
    goto <bb 5>; [15.00%]

  <bb 3> [12.75%]:
  ivtmp.9_7 = (unsigned long) &MEM[(void *)&__bss_start1 + 8B];
  _12 = (unsigned long) &__bss_end1;
  _13 = _12 + 7;
  _16 = _13 - ivtmp.9_7;
  _17 = _16 & 18446744073709551608;
  _18 = (unsigned long) &__bss_start1;
  _19 = _18 + 16;
  _20 = _17 + _19;

  <bb 4> [85.00%]:
  # ivtmp.9_10 = PHI <ivtmp.9_1(4), ivtmp.9_7(3)>
  _11 = (void *) ivtmp.9_10;
  MEM[base: _11, offset: -8B] = 0;
  ivtmp.9_1 = ivtmp.9_10 + 8;
  if (ivtmp.9_1 != _20)
    goto <bb 4>; [85.00%]
  else
    goto <bb 5>; [15.00%]

  <bb 5> [15.00%]:
  return;

}


;; Function clear_bss2 (clear_bss2, funcdef_no=1, decl_uid=3127, cgraph_uid=1, symbol_order=1)

Removing basic block 5
Removing basic block 6
Removing basic block 7
Removing basic block 8
clear_bss2 ()
{
  long unsigned int * bss;
  long unsigned int * __bss_end2.2_10;

  <bb 2> [15.00%]:
  bss_5 = __bss_start2;
  __bss_end2.2_10 = __bss_end2;
  if (bss_5 < __bss_end2.2_10)
    goto <bb 3>; [85.00%]
  else
    goto <bb 4>; [15.00%]

  <bb 3> [85.00%]:
  # bss_11 = PHI <bss_6(3), bss_5(2)>
  bss_6 = bss_11 + 8;
  MEM[base: bss_6, offset: -8B] = 0;
  if (bss_6 < __bss_end2.2_10)
    goto <bb 3>; [85.00%]
  else
    goto <bb 4>; [15.00%]

  <bb 4> [15.00%]:
  return;

}





Is this helping to explain my bug entry?


best regards,

Florian La Roche

Right, compiling with "-O2 -fno-ivopts" resolves my issues.

best regards,

Florian La Roche

I suspect this is due to what looks like undefined code.  That is you cannot compare two different arrays and expect good behavior.

Yeah.  You probably need something like:
void clear_bss1(void)
{
    unsigned long *bss = __bss_start1;
    asm ("" : "+g" (bss));
    while (bss < __bss_end1)
        *bss++ = 0UL;
}
to hide the UB from the optimizers; pointer arithmetics is only defined within the same object and similarly pointer comparison other than ==/!=, while the first function has two different objects.

Hello Andrew Pinski,

shouldn't the compiler see that both must be aligned to 8 bytes
and thus also their difference must be a multiple of 8 bytes?

I haven't looked into gcc sources, but maybe this information could
be exploited for additinal optimization.

best regards,

Florian La Roche

I've found something the compiler optimized quite nicely:
(Good for the compiler, but I'd be happy to stay with the original code
that was much easier to read for humans.)



extern unsigned long __bss_start[];
extern unsigned long __bss_end[];
//extern unsigned long __bss_size;

void clear_bss(void)
{
    unsigned long *bss = __bss_start;
    unsigned long i, end = __bss_end - __bss_start;
    //unsigned long i = __bss_size;
    for (i = 0; i < end; i += sizeof (unsigned long))
        *bss++ = 0UL;
}




This results on aarch64 into this code:
0000000000000000 <clear_bss>:
   0:	90000001 	adrp	x1, 0 <__bss_end>
   4:	90000002 	adrp	x2, 0 <__bss_start>
   8:	f9400021 	ldr	x1, [x1]
   c:	f9400042 	ldr	x2, [x2]
  10:	cb020021 	sub	x1, x1, x2
  14:	9343fc21 	asr	x1, x1, #3
  18:	b40000c1 	cbz	x1, 30 <clear_bss+0x30>
  1c:	d2800000 	mov	x0, #0x0                   	// #0
  20:	f822681f 	str	xzr, [x0, x2]
  24:	91002000 	add	x0, x0, #0x8
  28:	eb00003f 	cmp	x1, x0
  2c:	54ffffa8 	b.hi	20 <clear_bss+0x20>  // b.pmore
  30:	d65f03c0 	ret


Jakub, your example code did also result in pretty large code
(but I've only tested 8.0.1, not the newest release on this).


Thanks a lot,
best regards,

Florian La Roche

Puh, even introduced an error here. This one works, but is
getting complex compared to the original code:



extern unsigned long __bss_start[];
extern unsigned long __bss_end[];

void clear_bss(void)
{
    unsigned long *bss = __bss_start;
    unsigned long i, end = (__bss_end - __bss_start) * sizeof (unsigned long);
    for (i = 0; i < end; i += sizeof (unsigned long))
        *bss++ = 0UL;
}


best regards,

Florian La Roche

In my optionion the result of
"end = (__bss_end - __bss_start) * sizeof (unsigned long)"
in my last testcase should show that the compile should be
able to optimize the test code of the original submitted code.

(Still of course completely unclear if this makes sense to implement.)

best regards,

Florian La Roche

Below my current code that disables optimization for this one function and thus
generates ok code length.

best regards,

Florian La Roche




#if __GNUC__ > 4
#define __gcc_no_ivopts __attribute__ ((optimize("no-ivopts")))
#else
#define __gcc_no_ivopts
#endif

extern unsigned long __bss_start[], __bss_end[];

void __gcc_no_ivopts clear_bss(void)
{
    unsigned long *bss = __bss_start;
#if 1
    while (bss < __bss_end)
        *bss++ = 0UL;
#else
    unsigned long i, end = (__bss_end - __bss_start) * sizeof(unsigned long);
    for (i = 0; i < end; i += sizeof(unsigned long))
        *bss++ = 0UL;
#endif
}

These days we get:
  _1 = (unsigned long) &__bss_end1;
  _7 = _1 + 18446744073709551615;
  _6 = (unsigned long) &__bss_start1;
  _11 = _7 - _6;
  _12 = _11 >> 3;
  _13 = _12 + 1;
  _14 = _13 * 8;
  __builtin_memset (&__bss_start1, 0, _14); [tail call]


Or
((((end - 1) - start) >> 3) + 1) * 8

In theory (I think) we could do:
(end - start + 7) & -8