Re: bug report x86-64 code: je / jbe

2023-06-25 Thread Iain Buclaw via D.gnu

On Monday, 19 June 2023 at 09:54:32 UTC, Iain Buclaw wrote:

On Sunday, 18 June 2023 at 02:55:32 UTC, Cecil Ward wrote:

On Friday, 16 June 2023 at 13:12:06 UTC, Iain Buclaw wrote:


Redundant jump? Yes, arguably. Leads to wrong runtime? 
Doesn't look that way.


Completely agree with Iain, it’s not incorrect code, I wasn’t 
intending to suggest that. I’d just say suboptimal, and not 
the very best code generation possible.


By the way, I suspect it's related to this problem report.

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96435



Fix has been committed in time for 13.2 release.


Re: bug report x86-64 code: je / jbe

2023-06-19 Thread Iain Buclaw via D.gnu

On Sunday, 18 June 2023 at 02:55:32 UTC, Cecil Ward wrote:

On Friday, 16 June 2023 at 13:12:06 UTC, Iain Buclaw wrote:


Redundant jump? Yes, arguably. Leads to wrong runtime? Doesn't 
look that way.


Completely agree with Iain, it’s not incorrect code, I wasn’t 
intending to suggest that. I’d just say suboptimal, and not the 
very best code generation possible.


By the way, I suspect it's related to this problem report.

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96435

Taken from upstream DMD comment (I suspect is still not fixed in 
either DMD, possibly LDC too but haven't checked).


https://issues.dlang.org/show_bug.cgi?id=20148#c3

Essentially, every read of a boolean gets turned into 
`(*(ubyte*)) & 1`.


This *could* be limited to `@safe` code, or just accessing field 
members of a union I suppose.


Re: bug report x86-64 code: je / jbe

2023-06-17 Thread Cecil Ward via D.gnu

On Friday, 16 June 2023 at 13:12:06 UTC, Iain Buclaw wrote:

On Wednesday, 14 June 2023 at 12:35:43 UTC, Cecil Ward wrote:
I have just noticed a bug in the latest release of GDC that 
targets x86-64. For example GDC 12.3 and above versions too, 
running on X86-64, targeting self. This was built with:

 -O3 -frelease -march=alderlake



What leads you to believe that it is buggy?


Generated code is:

   je L1
   jbe L1



What I see is the first instruction is going to relate to this 
condition.



if ( unlikely( p == 1 ) ) return x;


Then the next instruction is the condition in the following 
for-loop.



for ( exp_ui_t i = p; i > 1; i >>= 1 )


Redundant jump? Yes, arguably. Leads to wrong runtime? Doesn't 
look that way.


Completely agree with Iain, it’s not incorrect code, I wasn’t 
intending to suggest that. I’d just say suboptimal, and not the 
very best code generation possible.


Re: bug report x86-64 code: je / jbe

2023-06-16 Thread Iain Buclaw via D.gnu

On Wednesday, 14 June 2023 at 12:35:43 UTC, Cecil Ward wrote:
I have just noticed a bug in the latest release of GDC that 
targets x86-64. For example GDC 12.3 and above versions too, 
running on X86-64, targeting self. This was built with:

 -O3 -frelease -march=alderlake



What leads you to believe that it is buggy?


Generated code is:

   je L1
   jbe L1



What I see is the first instruction is going to relate to this 
condition.



if ( unlikely( p == 1 ) ) return x;


Then the next instruction is the condition in the following 
for-loop.



for ( exp_ui_t i = p; i > 1; i >>= 1 )


Redundant jump? Yes, arguably. Leads to wrong runtime? Doesn't 
look that way.


bug report x86-64 code: je / jbe

2023-06-14 Thread Cecil Ward via D.gnu
I have just noticed a bug in the latest release of GDC that 
targets x86-64. For example GDC 12.3 and above versions too, 
running on X86-64, targeting self. This was built with:

 -O3 -frelease -march=alderlake

Generated code is:

   je L1
   jbe L1

…
…
L1:  ret

The probably reason for this is my use of the GDC built in for 
indicating whether conditional jumps are likely or unlikely to be 
taken. I wrote a trivial routine likely() and unlikely() and used 
this as follows:


public
T pow(T, exp_ui_t )( in T x, in exp_ui_t p ) pure @safe nothrow 
@nogc

if ( is ( exp_ui_t == ulong ) || is ( exp_ui_t == uint ) )
in  {
static assert( is ( typeof( x * x ) ) );
assert( p >= 0 );
}
out ( ret ) {
assert( ( p == 0 && ret == 1 ) || !( p == 0 ) );
}
do
{
if ( unlikely( p == 0 ) ) return 1;
if ( unlikely( p == 1 ) ) return x;

/*
if ( unlikely( x == 0 ) )   // fast-path opt, unnecessary
return x;
if ( unlikely( x == 1 ) )   // fast-path opt, unnecessary
return x;
*/

T s = x;
T v = 1;

for ( exp_ui_t i = p; i > 1; i >>= 1 )
{
v = ( i & 0x1 ) ? s * v : v;
s = s * s;
}
   //assert( p > 1 && pow( x, p ) == ( p > 1 ? x * pow( x, p-1) : 
1) );

return v * s;
}

pragma( inline, true )
private
bool builtin_expect()( in bool test_cond, in bool expected_cond ) 
 pure nothrow @safe @nogc

{
version ( LDC )
	{// ldc.intrinsics.llvm_expect - didi not seem to work when 
tested in LDC 1.22

import ldc.intrinsics : llvm_expect;
return cast(bool) llvm_expect( test_cond, expected_cond );
}
version ( GDC )
{
import gcc.builtins : __builtin_expect;
	return cast(bool) __builtin_expect( test_cond, expected_cond 
);

}
return test_cond;
}


pragma( inline, true )
public
bool likely()( in bool test_cond ) pure nothrow @safe @nogc
/* Returns test_cond which makes it convenient to do assert( 
unlikely() )
 * Also emulates builtin_expect's return behaviour, by returning 
the argument

 */ {
return builtin_expect( test_cond, true );
}


pragma( inline, true )
public
bool unlikely()( in bool test_cond ) pure nothrow @safe @nogc
/* Returns test_cond which makes it convenient to do assert( 
unlikely() )
 * Also emulates builtin_expect's return behaviour, by returning 
the argument

 */
{
return builtin_expect( test_cond, false );
}
// ~~~ module likely - end.



This is not the whole of this .d file, I can of course give you 
the whole lot if you desire. I inspected the result in Matt 
Godbolt’s compiler explorer website godbolt.org.


An aside: LDC:: I need to look at LDC’s llvm_expect to see if it 
is controlling the branches the way I wish. Does anyone know if 
llvm_expect has any problems?