benchmark/common: using argument-provided location for MEM_AREAs
Using a lot the stack to save these values...
Merge request reports
Activity
- Resolved by DERUMIGNY Nicolas
1222 out.comment("clear memory arena") 1233 out.comment("clear mem_store_area") 1234 # FIXME: other address sizes 1223 1235 ## void *memset(void *s, int c, size_t n); 1224 1236 s = out.get_argument_register(0) 1225 1237 c = out.get_argument_register(1) 1226 1238 n = out.get_argument_register(2) 1227 out.put_const_in_register(MEMORY_ARENA_LD, s) 1239 MEMORY_REG_STORE = out.ir_builder.select_memory_base_register( 1240 benchmark.instructions, 1241 set(out.iter_free_registers()), 1242 64, 1243 ) 1244 out.comment("mem_store -> ", MEMORY_REG_STORE) 1245 # popping mem_store_area from the stack 1246 out.pop_from_stack(s) I'm pretty sure this is way overkill. If I get you right, at this point, the top-value on the stack is a pointer to the memory region, which you want to pass as 1st parameter for
memset
, and for this, youpopq %rdi
, move the value to some register (?), push it back on the stack, and callmemset
which still has this address loaded as%rdi
(1st argument).Wouldn't a
mov (%rsp), %rdi
be more on point?I agree; but there is currently no support for high-level generation of mov from memory to register, hence the multiple push/pop (and I am not sure what cost it will have when moving to ARM support). I furthermore believe that these stack changes will stay in the L/S queue and therefore will not harm performances, if
as
is not already optimizing after us.
- Resolved by Théophile BASTIAN
- Resolved by Théophile BASTIAN
I'm really not sure at all I got everything right in this review, this codebase is still quite obscure to me. Don't trust my review too much.
added 1 commit
- f3a9a831 - benchmakr/common: updating according to reviews
mentioned in commit b5782fef
mentioned in commit CORSE/pipedream@59948da2