>
> Bootstrap and gcc-check are passing without regression.

Sorry, it's taken me a while to get to this but I looked at this for
sometime last night and think this needs a bit of work .

1. Can the code for TARGET_APCS_FRAME be factored out into a separate
function from arm_expand_epilogue ?

   Essentially factor out from arm_expand_epilogue:

 if (TARGET_APCS_FRAME && frame_pointer_needed && TARGET_ARM)
+    {
.......

+    }

2. I'm not happy about the amount of very similar code between ARM and
Thumb2 state as far as arm_expand_return / thumb2_expand_return and
arm_expand_epilogue / thumb2_expand_epilogue go. Could you try to
merge the 2 together into the same functions.

3. The return sequence of loading into PC from the stack has
disappeared . A simple test appeared to generate :

        @ sp needed for prologue
        ldr     lr, [sp], #4
        bx      lr

instead of ldmfd sp!, {pc} atleast in the single return cases.

Which is still correct but sub-optimal.

4. A number of cases around *cond_return and *cond_return_inverted are
removed - This removes the chance for getting conditional returns from
the compiler - marking the return pattern as predicable doesn't really
generate conditional returns you really need the (if_then_else form).


5. This hunk here :

@@ -22741,6 +22646,7 @@ thumb2_expand_epilogue (void)
              return-address' instruction.  Instead, pop LR in PC.  */
           if (ARM_FUNC_TYPE (func_type) == ARM_FT_NORMAL
               && !IS_STACKALIGN (func_type)
+              && !is_sibling
               && crtl->args.pretend_args_size == 0
               && saved_regs_mask & (1 << LR_REGNUM)
               && !crtl->calls_eh_return)

This would be better off in the original Thumb2 RTL epilogues patch ?


6. This has missed out the hunk that deals with a ARM10 VFPr1 bug --

-  /* Workaround ARM10 VFPr1 bug.  */
-  if (count == 2 && !arm_arch6)
-    {
-      if (reg == 15)
-       reg--;
-      count++;
-    }

I'm not sure how many cores had this issue in the wild, but the
compiler has never generated things that went across from d15-d16 - We
seem to have always ended up loading something more into one of the
registers.

Next time please also submit a changelog entry -  it makes it easier
to check and review as well. Its largely similar to your original
changelog but it's easier to find that in the mail with the patch
rather than ferret around in older mails.


cheers
Ramana

Reply via email to