On 2023/01/06 6:32, Max Filippov wrote: > Hi Suwa-san, Hi! > > On Thu, Jan 5, 2023 at 3:57 AM Takayuki 'January June' Suwa > <jjsuwa_sys3...@yahoo.co.jp> wrote: >> >> This patch introduces a convenient helper function for integer immediate >> addition with scratch register as needed, that splits and emits either >> up to two ADDI/ADDMI machine instructions or an addition by register >> following an immediate integer load (which may later be transformed by >> constantsynth). >> >> By using the helper function, it makes stack frame adjustment logic >> simplified and instruction count less in some cases. >> >> gcc/ChangeLog: >> >> * config/xtensa/xtensa.cc >> (xtensa_split_imm_two_addends, xtensa_emit_add_imm): >> New helper functions. >> (xtensa_emit_adjust_stack_ptr, xtensa_set_return_address, >> xtensa_output_mi_thunk): Change to use the helper function. >> --- >> gcc/config/xtensa/xtensa.cc | 139 +++++++++++++++++++++++------------- >> 1 file changed, 88 insertions(+), 51 deletions(-) > > This change introduces a bunch of failures in the g++ testsuite, > but the culprit is apparently somewhere in the libstdc++.so, I'm > still looking for it. > > I see the following pattern change in the generated epilogue code: > > - 4aaf: b0a192 movi a9, 0x1b0 > - 4ab2: 1f9a add.n a1, a15, a9 > ... > - 4abe: 20c112 addi a1, a1, 32 > - 4ac1: f00d ret.n > + 4aaf: 02df12 addmi a1, a15, 0x200 > + 4ab2: b0c112 addi a1, a1, -80 > ... > + 4abf: 20c112 addi a1, a1, 32 > + 4ac2: f00d ret.n > > I.e. a1 is first moved into the parent stack frame, then back to the right > spot. This does not look correct, especially for bare-metal targets. >
On second thought, it cannot be a good idea to split addition/subtraction to the stack pointer. > - 4aaf: b0a192 movi a9, 0x1b0 > - 4ab2: 1f9a add.n a1, a15, a9 > + 4aaf: 02df12 addmi a1, a15, 0x200 > + 4ab2: b0c112 addi a1, a1, -80 Because the former is atomic, but the latter is not. (may be interrupted between the two add instructions) I'll wait for the results of your investigation, but it may be better to withdraw the patch.
