D.gnu - bug report x86-64 code: je / jbe
- Cecil Ward (96/96) Jun 14 2023 I have just noticed a bug in the latest release of GDC that
- Iain Buclaw (8/17) Jun 16 2023 What I see is the first instruction is going to relate to this
- Cecil Ward (4/24) Jun 17 2023 Completely agree with Iain, it’s not incorrect code, I wasn’t
- Iain Buclaw (10/17) Jun 19 2023 By the way, I suspect it's related to this problem report.
- Iain Buclaw (2/13) Jun 25 2023 Fix has been committed in time for 13.2 release.
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?
Jun 14 2023
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=alderlakeWhat leads you to believe that it is buggy?Generated code is: je L1 jbe L1What 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.
Jun 16 2023
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: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.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=alderlakeWhat leads you to believe that it is buggy?Generated code is: je L1 jbe L1What 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.
Jun 17 2023
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: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*)&b) & 1`. This *could* be limited to ` safe` code, or just accessing field members of a union I suppose.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.
Jun 19 2023
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:Fix has been committed in time for 13.2 release.On Friday, 16 June 2023 at 13:12:06 UTC, Iain Buclaw wrote:By the way, I suspect it's related to this problem report. https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96435Redundant 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.
Jun 25 2023