Hello Bernhard: On 23/10/23 7:40 pm, Bernhard Reutner-Fischer wrote: > On Mon, 23 Oct 2023 12:16:18 +0530 > Ajit Agarwal <aagar...@linux.ibm.com> wrote: > >> Hello All: >> >> Addressed below review comments in the version 11 of the patch. >> Please review and please let me know if its ok for trunk. > > s/satisified/satisfied/ >
I will fix this. >>> As said, I don't see why the below was not cleaned up before the V1 >>> submission. >>> Iff it breaks when manually CSEing, I'm curious why? > > The function below looks identical in v12 of the patch. > Why didn't you use common subexpressions? > ba Using CSE here breaks aarch64 regressions hence I have reverted it back not to use CSE, >>> >>>>> +/* Return TRUE if reg source operand of zero_extend is argument registers >>>>> + and not return registers and source and destination operand are same >>>>> + and mode of source and destination operand are not same. */ >>>>> + >>>>> +static bool >>>>> +abi_extension_candidate_p (rtx_insn *insn) >>>>> +{ >>>>> + rtx set = single_set (insn); >>>>> + machine_mode dst_mode = GET_MODE (SET_DEST (set)); >>>>> + rtx orig_src = XEXP (SET_SRC (set), 0); >>>>> + >>>>> + if (!FUNCTION_ARG_REGNO_P (REGNO (orig_src)) >>>>> + || abi_extension_candidate_return_reg_p (/*insn,*/ REGNO >>>>> (orig_src))) >>>>> + return false; >>>>> + >>>>> + /* Mode of destination and source should be different. */ >>>>> + if (dst_mode == GET_MODE (orig_src)) >>>>> + return false; >>>>> + >>>>> + machine_mode mode = GET_MODE (XEXP (SET_SRC (set), 0)); >>>>> + bool promote_p = abi_target_promote_function_mode (mode); >>>>> + >>>>> + /* REGNO of source and destination should be same if not >>>>> + promoted. */ >>>>> + if (!promote_p && REGNO (SET_DEST (set)) != REGNO (orig_src)) >>>>> + return false; >>>>> + >>>>> + return true; >>>>> +} >>>>> + > > >>> >>> As said, please also rephrase the above (and everything else if it >>> obviously looks akin the above). > > thanks