On Fri, Mar 7, 2014 at 10:24 AM, Christian Bruel <christian.br...@st.com> wrote: > Hi Ramana, > > Thanks for your comments, > >> Please respin using plus_constant instead of gen_addsi3. > > Here is my feeling about this: > > I experimented on using plus_constant instead of gen_addsi3. But there > are cases when the emitted code is not equivalent for large frames > (!const_ok_for_op (val, PLUS)) and leads to complications.
Ah, Yes you are right. I hadn't remembered that when I looked at it yesterday. > > We could fix this case with a call to arm_split_constant (PLUS, Pmode, > NULL, amount, stack_pointer_rtx, stack_pointer_rtx, 0), but I'm not > sure we gain in clarity here. Also for consistency, the same interface > change would preferably be needed in the other parts of the arm.c file > (that I didn't modify) sharing the same sequence. For instance > "arm_expand_epilogue" > Agreed, There's no need to do that given your explanation. Looks good to me - please give an RM 24 working hours i.e. till end of day on 11th March to comment before committing this. Please also remove mfloat-abi=hard from pr60264.c, this will just conflict when people test for Thumb1 or with other conflicting multilibs i.e. with soft-float. There are enough autotesters with the hardfloat abi these days that we'll find any regressions that might be there. regards Ramana >> Otherwise >> this looks good to me. >> >> Please repost updated patch and I will look at it again. > > For the reasons expressed above it'd prefer to consider this new change > as a separate development with a new patch. > > For the time being (considering only the original apcs issue) is it OK > to apply only the patch as it ? and validate a global > gen_addsi3/plus_constant interface review as a separate step ? > > Many thanks, > > Christian > >> >> Ramana >
