firmware: Minor optimization in _scratch_init()
PATCH 1/1 firmware: Minor optimization in _scratch_init()
Before entering _scratch_init(), register t3 already holds a copy of the firmware end address, hence there is no need to calculate it again. This reduces 3 instructions in each _scratch_init() loop.
---
firmware/fw_base.S | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-)
firmware/fw_base.S +1 -4
@@ -287,10 +287,7 @@ _scratch_init: /* Initialize scratch space */ /* Store fw_start and fw_size in scratch space */ lla a4, _fw_start - lla a5, _fw_end - mul t0, s7, s8 - add a5, a5, t0 - sub a5, a5, a4 + sub a5, t3, a4 REG_S a4, SBI_SCRATCH_FW_START_OFFSET(tp) REG_S a5, SBI_SCRATCH_FW_SIZE_OFFSET(tp) /* Store next arg1 in scratch space */
Signature · 1 line
2.25.1
PATCH 1/1 firmware: Minor optimization in _scratch_init()
? 2021-07-08???? 21:35 +0800?Bin Meng???
Quoted 5 lines
Before entering
_scratch_init(), register t3 already holds a copy of the firmware end address, hence there is no need to calculate it again. This reduces 3 instructions in each_scratch_init()loop.Signed-off-by Bin Meng <[email protected]>
Quoted 25 lines
---
?firmware/fw_base.S | 5 +----
?1 file changed, 1 insertion(+), 4 deletions(-)firmware/fw_base.S +0
-0@@ -287,10 +287,7 @@ _scratch_init:????????/* Initialize scratch space */ ????????/* Store
fw_startandfw_sizein scratch space */ ????????lla?????a4,_fw_start-???????lla?????a5,_fw_end
-???????mul?????t0, s7, s8
-???????add?????a5, a5, t0
-???????sub?????a5, a5, a4
+???????sub?????a5, t3, a4
????????REG_S???a4,SBI_SCRATCH_FW_START_OFFSET(tp)????????REG_S???a5,SBI_SCRATCH_FW_SIZE_OFFSET(tp)????????/* Store next arg1 in scratch space */Signature · 1 line
2.25.1
PATCH 1/1 firmware: Minor optimization in _scratch_init()
In reply to Bin MengQuoted 16 lines · Bin Meng
Before entering
_scratch_init(), register t3 already holds a copy of the firmware end address, hence there is no need to calculate it again. This reduces 3 instructions in each_scratch_init()loop.Signed-off-by Bin Meng <[email protected]>---
firmware/fw_base.S | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-)firmware/fw_base.S +0
-0
firmware/fw_base.S _scratch_init: · line 290
Quoted 8 lines
firmware/fw_base.S +1
-4@@ -287,10 +287,7 @@ _scratch_init: /* Initialize scratch space */ /* Store fw_start and fw_size in scratch space */ lla a4, _fw_start - lla a5, _fw_end - mul t0, s7, s8 - add a5, a5, t0 - sub a5, a5, a4 + sub a5, t3, a4
It saves 3 instructions during boot but uses a temporary register (t3) which was not computed in this block.
The first instruction of the loop also uses t3 to save tp. Can you add a comment in the beginning of the loop
saying that all the registers that are critical for _scratch_init loop ?
This will remind the future developers that they shouldn't touch these register inside the loop.
Quoted 11 lines
firmware/fw_base.S +0
-1REG_S a4, SBI_SCRATCH_FW_START_OFFSET(tp) REG_S a5, SBI_SCRATCH_FW_SIZE_OFFSET(tp) /* Store next arg1 in scratch space */ --2.25.1
--
opensbi mailing list
opensbi at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi
Otherwise, looks good to me.
--
Regards,
Atish
PATCH 1/1 firmware: Minor optimization in _scratch_init()
Quoted 33 lines · Atish Patra
Quoted 16 lines · Bin Meng
Before entering
_scratch_init(), register t3 already holds a copy of the firmware end address, hence there is no need to calculate it again. This reduces 3 instructions in each_scratch_init()loop.Signed-off-by Bin Meng <[email protected]>---
firmware/fw_base.S | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-)firmware/fw_base.S +0
-0firmware/fw_base.S _scratch_init: · line 290
Quoted 8 lines
firmware/fw_base.S +1
-4@@ -287,10 +287,7 @@ _scratch_init: /* Initialize scratch space */ /* Store fw_start and fw_size in scratch space */ lla a4, _fw_start - lla a5, _fw_end - mul t0, s7, s8 - add a5, a5, t0 - sub a5, a5, a4 + sub a5, t3, a4It saves 3 instructions during boot but uses a temporary register (t3) which was not computed in this block.
The first instruction of the loop also uses t3 to save tp. Can you add a comment in the beginning of the loop
saying that all the registers that are critical for_scratch_initloop ?
Quoted 2 lines
This will remind the future developers that they shouldn't touch these register inside the loop.
Sure, will add a comment in v2.
Quoted 9 lines
Quoted 9 lines
firmware/fw_base.S +0
-1REG_S a4, SBI_SCRATCH_FW_START_OFFSET(tp) REG_S a5, SBI_SCRATCH_FW_SIZE_OFFSET(tp) /* Store next arg1 in scratch space */ --2.25.1
Quoted 8 lines
Quoted 4 lines
--
opensbi mailing list
opensbi at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbiOtherwise, looks good to me.
Reviewed-by Atish Patra <[email protected]>
Regards,
Bin