Fix-Point commented on code in PR #17640:
URL: https://github.com/apache/nuttx/pull/17640#discussion_r2644581980
##########
sched/wdog/wd_start.c:
##########
@@ -265,62 +258,63 @@ int wd_start_abstick(FAR struct wdog_s *wdog, clock_t
ticks,
wdentry_t wdentry, wdparm_t arg)
{
irqstate_t flags;
- bool reassess = false;
+ bool reassess = false;
+ int ret = -EINVAL;
/* Verify the wdog and setup parameters */
- if (wdog == NULL || wdentry == NULL)
+ if (wdog != NULL && wdentry != NULL)
Review Comment:
According to the test results, the modified code performs better.
I constructed a simple test program as follows:
```c
int fa(int *a, int *b)
{
int ret = -1;
if (a != NULL && b != NULL) {
printf("a = %d, b = %d\n", *a, *b);
ret = 0;
}
return ret;
}
int fb(int *a, int *b)
{
if (a == NULL || b == NULL) return -1;
printf("a = %d, b = %d\n", *a, *b);
return 0;
}
```
The two functions are logically equivalent.
To actually verify which implementation performs better, we used CompCert C
3.15 (a fully formally verified safety compiler) with O3 optimization, and
compiled it into the following assembly code:
```bash
00000000004005c4 <fa>:
4005c4: 910003ef mov x15, sp
4005c8: d10043ff sub sp, sp, #0x10
4005cc: f90003ef str x15, [sp]
4005d0: f90007fe str x30, [sp, #8]
4005d4: 12800002 mov w2, #0xffffffff //
#-1
4005d8: b4000140 cbz x0, 400600 <fa+0x3c>
4005dc: b4000121 cbz x1, 400600 <fa+0x3c>
4005e0: 90000003 adrp x3, 400000 <__abi_tag-0x278>
4005e4: 911be863 add x3, x3, #0x6fa
4005e8: b9400000 ldr w0, [x0]
4005ec: b9400022 ldr w2, [x1]
4005f0: aa0003e1 mov x1, x0
4005f4: aa0303e0 mov x0, x3
4005f8: 97ffffaa bl 4004a0 <printf@plt>
4005fc: 52800002 mov w2, #0x0 // #0
400600: aa0203e0 mov x0, x2
400604: f94007fe ldr x30, [sp, #8]
400608: 910043ff add sp, sp, #0x10
40060c: d65f03c0 ret
0000000000400610 <fb>:
400610: 910003ef mov x15, sp
400614: d10043ff sub sp, sp, #0x10
400618: f90003ef str x15, [sp]
40061c: f90007fe str x30, [sp, #8]
400620: aa0103e2 mov x2, x1
400624: aa0003e1 mov x1, x0
400628: b4000121 cbz x1, 40064c <fb+0x3c>
40062c: b4000102 cbz x2, 40064c <fb+0x3c>
400630: 90000000 adrp x0, 400000 <__abi_tag-0x278>
400634: 911be800 add x0, x0, #0x6fa
400638: b9400021 ldr w1, [x1]
40063c: b9400042 ldr w2, [x2]
400640: 97ffff98 bl 4004a0 <printf@plt>
400644: 52800000 mov w0, #0x0 // #0
400648: 14000002 b 400650 <fb+0x40>
40064c: 12800000 mov w0, #0xffffffff //
#-1
400650: f94007fe ldr x30, [sp, #8]
400654: 910043ff add sp, sp, #0x10
400658: d65f03c0 ret
```
We found that the former has one less unconditional jump than the latter:
```bash
400648: 14000002 b 400650 <fb+0x40>
```
This makes the former perform better.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
