| Message ID | 57FE0F22.2020209@foss.arm.com |
|---|---|
| State | New |
| Headers | show |
Ping. https://gcc.gnu.org/ml/gcc-patches/2016-10/msg00839.html Thanks, Kyrill On 12/10/16 11:23, Kyrill Tkachov wrote: > Hi all, > > I'm looking at the aarch64 prologue and epilogue generation code and I noticed many of the helper > functions don't have function comments so it makes it harder than it has to to understand what's going on. > This patch adds function comments to some of them. I hope I understood the functions correctly. > > Is this ok for trunk? > > Thanks, > Kyrill > > 2016-10-12 Kyrylo Tkachov <kyrylo.tkachov@arm.com> > > * config/aarch64/aarch64.c (aarch64_register_saved_on_entry): Add > function comment. > (aarch64_next_callee_save): Likewise. > (aarch64_pushwb_single_reg): Likewise. > (aarch64_gen_storewb_pair): Likewise. > (aarch64_push_regs): Likewise. > (aarch64_gen_loadwb_pair): Likewise. > (aarch64_pop_regs): Likewise. > (aarch64_gen_store_pair): Likewise. > (aarch64_gen_load_pair): Likewise. > (aarch64_save_callee_saves): Likewise. > (aarch64_restore_callee_saves): Likewise.
Ping. Thanks, Kyrill On 24/10/16 12:30, Kyrill Tkachov wrote: > Ping. > https://gcc.gnu.org/ml/gcc-patches/2016-10/msg00839.html > > Thanks, > Kyrill > > On 12/10/16 11:23, Kyrill Tkachov wrote: >> Hi all, >> >> I'm looking at the aarch64 prologue and epilogue generation code and I noticed many of the helper >> functions don't have function comments so it makes it harder than it has to to understand what's going on. >> This patch adds function comments to some of them. I hope I understood the functions correctly. >> >> Is this ok for trunk? >> >> Thanks, >> Kyrill >> >> 2016-10-12 Kyrylo Tkachov <kyrylo.tkachov@arm.com> >> >> * config/aarch64/aarch64.c (aarch64_register_saved_on_entry): Add >> function comment. >> (aarch64_next_callee_save): Likewise. >> (aarch64_pushwb_single_reg): Likewise. >> (aarch64_gen_storewb_pair): Likewise. >> (aarch64_push_regs): Likewise. >> (aarch64_gen_loadwb_pair): Likewise. >> (aarch64_pop_regs): Likewise. >> (aarch64_gen_store_pair): Likewise. >> (aarch64_gen_load_pair): Likewise. >> (aarch64_save_callee_saves): Likewise. >> (aarch64_restore_callee_saves): Likewise. >
On 31/10/16 12:10, Kyrill Tkachov wrote: > Ping. > > Thanks, > Kyrill > > On 24/10/16 12:30, Kyrill Tkachov wrote: >> Ping. >> https://gcc.gnu.org/ml/gcc-patches/2016-10/msg00839.html >> >> Thanks, >> Kyrill >> >> On 12/10/16 11:23, Kyrill Tkachov wrote: >>> Hi all, >>> >>> I'm looking at the aarch64 prologue and epilogue generation code and >>> I noticed many of the helper >>> functions don't have function comments so it makes it harder than it >>> has to to understand what's going on. >>> This patch adds function comments to some of them. I hope I >>> understood the functions correctly. >>> >>> Is this ok for trunk? >>> >>> Thanks, >>> Kyrill >>> >>> 2016-10-12 Kyrylo Tkachov <kyrylo.tkachov@arm.com> >>> >>> * config/aarch64/aarch64.c (aarch64_register_saved_on_entry): Add >>> function comment. >>> (aarch64_next_callee_save): Likewise. >>> (aarch64_pushwb_single_reg): Likewise. >>> (aarch64_gen_storewb_pair): Likewise. >>> (aarch64_push_regs): Likewise. >>> (aarch64_gen_loadwb_pair): Likewise. >>> (aarch64_pop_regs): Likewise. >>> (aarch64_gen_store_pair): Likewise. >>> (aarch64_gen_load_pair): Likewise. >>> (aarch64_save_callee_saves): Likewise. >>> (aarch64_restore_callee_saves): Likewise. I "contributed" some of these functions without comments... The new added comments looks good to me though I can't approve. Thanks for fixing these. Regards, Jiong
On Tue, Nov 01, 2016 at 10:49:10AM +0000, Jiong Wang wrote: > >>>Is this ok for trunk? > >>> > >>>Thanks, > >>>Kyrill > >>> > >>>2016-10-12 Kyrylo Tkachov <kyrylo.tkachov@arm.com> > >>> > >>> * config/aarch64/aarch64.c (aarch64_register_saved_on_entry): Add > >>> function comment. > >>> (aarch64_next_callee_save): Likewise. > >>> (aarch64_pushwb_single_reg): Likewise. > >>> (aarch64_gen_storewb_pair): Likewise. > >>> (aarch64_push_regs): Likewise. > >>> (aarch64_gen_loadwb_pair): Likewise. > >>> (aarch64_pop_regs): Likewise. > >>> (aarch64_gen_store_pair): Likewise. > >>> (aarch64_gen_load_pair): Likewise. > >>> (aarch64_save_callee_saves): Likewise. > >>> (aarch64_restore_callee_saves): Likewise. > > I "contributed" some of these functions without comments... > The new added comments looks good to me though I can't approve. > > Thanks for fixing these. Thanks Jiong, I appreciate you taking the time to look. This is OK based on Jiong's technical review and after a glance to ensure the comments made sense to me. Thanks, James
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c index e7556632901177c04f9884be4f3ee40e5f677917..87bb5407245a7e902a2153a3ed963cead3aa4bcc 100644 --- a/gcc/config/aarch64/aarch64.c +++ b/gcc/config/aarch64/aarch64.c @@ -2896,12 +2896,18 @@ aarch64_layout_frame (void) cfun->machine->frame.laid_out = true; } +/* Return true if the register REGNO is saved on entry to + the current function. */ + static bool aarch64_register_saved_on_entry (int regno) { return cfun->machine->frame.reg_offset[regno] >= 0; } +/* Return the next register up from REGNO up to LIMIT for the callee + to save. */ + static unsigned aarch64_next_callee_save (unsigned regno, unsigned limit) { @@ -2910,6 +2916,9 @@ aarch64_next_callee_save (unsigned regno, unsigned limit) return regno; } +/* Push the register number REGNO of mode MODE to the stack with write-back + adjusting the stack by ADJUSTMENT. */ + static void aarch64_pushwb_single_reg (machine_mode mode, unsigned regno, HOST_WIDE_INT adjustment) @@ -2926,6 +2935,10 @@ aarch64_pushwb_single_reg (machine_mode mode, unsigned regno, RTX_FRAME_RELATED_P (insn) = 1; } +/* Generate and return an instruction to store the pair of registers + REG and REG2 of mode MODE to location BASE with write-back adjusting + the stack location BASE by ADJUSTMENT. */ + static rtx aarch64_gen_storewb_pair (machine_mode mode, rtx base, rtx reg, rtx reg2, HOST_WIDE_INT adjustment) @@ -2945,6 +2958,9 @@ aarch64_gen_storewb_pair (machine_mode mode, rtx base, rtx reg, rtx reg2, } } +/* Push registers numbered REGNO1 and REGNO2 to the stack, adjusting the + stack pointer by ADJUSTMENT. */ + static void aarch64_push_regs (unsigned regno1, unsigned regno2, HOST_WIDE_INT adjustment) { @@ -2964,6 +2980,9 @@ aarch64_push_regs (unsigned regno1, unsigned regno2, HOST_WIDE_INT adjustment) RTX_FRAME_RELATED_P (insn) = 1; } +/* Load the pair of register REG, REG2 of mode MODE from stack location BASE, + adjusting it by ADJUSTMENT afterwards. */ + static rtx aarch64_gen_loadwb_pair (machine_mode mode, rtx base, rtx reg, rtx reg2, HOST_WIDE_INT adjustment) @@ -2981,6 +3000,10 @@ aarch64_gen_loadwb_pair (machine_mode mode, rtx base, rtx reg, rtx reg2, } } +/* Pop the two registers numbered REGNO1, REGNO2 from the stack, adjusting it + afterwards by ADJUSTMENT and writing the appropriate REG_CFA_RESTORE notes + into CFI_OPS. */ + static void aarch64_pop_regs (unsigned regno1, unsigned regno2, HOST_WIDE_INT adjustment, rtx *cfi_ops) @@ -3005,6 +3028,9 @@ aarch64_pop_regs (unsigned regno1, unsigned regno2, HOST_WIDE_INT adjustment, } } +/* Generate and return a store pair instruction of mode MODE to store + register REG1 to MEM1 and register REG2 to MEM2. */ + static rtx aarch64_gen_store_pair (machine_mode mode, rtx mem1, rtx reg1, rtx mem2, rtx reg2) @@ -3022,6 +3048,9 @@ aarch64_gen_store_pair (machine_mode mode, rtx mem1, rtx reg1, rtx mem2, } } +/* Generate and regurn a load pair isntruction of mode MODE to load register + REG1 from MEM1 and register REG2 from MEM2. */ + static rtx aarch64_gen_load_pair (machine_mode mode, rtx reg1, rtx mem1, rtx reg2, rtx mem2) @@ -3039,6 +3068,9 @@ aarch64_gen_load_pair (machine_mode mode, rtx reg1, rtx mem1, rtx reg2, } } +/* Emit code to save the callee-saved registers from register number START + to LIMIT to the stack at the location starting at offset START_OFFSET, + skipping any write-back candidates if SKIP_WB is true. */ static void aarch64_save_callee_saves (machine_mode mode, HOST_WIDE_INT start_offset, @@ -3097,6 +3129,11 @@ aarch64_save_callee_saves (machine_mode mode, HOST_WIDE_INT start_offset, } } +/* Emit code to restore the callee registers of mode MODE from register + number START up to and including LIMIT. Restore from the stack offset + START_OFFSET, skipping any write-back candidates if SKIP_WB is true. + Write the appropriate REG_CFA_RESTORE notes into CFI_OPS. */ + static void aarch64_restore_callee_saves (machine_mode mode, HOST_WIDE_INT start_offset, unsigned start,
Hi all, I'm looking at the aarch64 prologue and epilogue generation code and I noticed many of the helper functions don't have function comments so it makes it harder than it has to to understand what's going on. This patch adds function comments to some of them. I hope I understood the functions correctly. Is this ok for trunk? Thanks, Kyrill 2016-10-12 Kyrylo Tkachov <kyrylo.tkachov@arm.com> * config/aarch64/aarch64.c (aarch64_register_saved_on_entry): Add function comment. (aarch64_next_callee_save): Likewise. (aarch64_pushwb_single_reg): Likewise. (aarch64_gen_storewb_pair): Likewise. (aarch64_push_regs): Likewise. (aarch64_gen_loadwb_pair): Likewise. (aarch64_pop_regs): Likewise. (aarch64_gen_store_pair): Likewise. (aarch64_gen_load_pair): Likewise. (aarch64_save_callee_saves): Likewise. (aarch64_restore_callee_saves): Likewise.