[LLVMbugs] [Bug 18081] New: [ARM Backend] Flag -Oz corrupted by folding SP update into push/pop

[bugzilla-daemon at llvm.org]

http://llvm.org/bugs/show_bug.cgi?id=18081

            Bug ID: 18081
           Summary: [ARM Backend] Flag -Oz corrupted by folding SP update
                    into push/pop
           Product: new-bugs
           Version: unspecified
          Hardware: PC
                OS: Linux
            Status: NEW
          Severity: normal
          Priority: P
         Component: new bugs
          Assignee: unassignedbugs at nondot.org
          Reporter: zhaoshiz at codeaurora.org
                CC: llvmbugs at cs.uiuc.edu, t.p.northover at gmail.com,
                    tim.northover at arm.com
    Classification: Unclassified

commit 323ac85d6ad7ba5d9593d8e151d879bd91d82e08
Author: Tim Northover <tnorthover at apple.com>
Date: Fri Nov 8 17:18:07 2013 +0000

  ARM: fold prologue/epilogue sp updates into push/pop for code size

There are two problems with this patch:

1. It cause a compile time assertion failure:
clang:
/local/mnt/workspace/llvm-arm/mainline-retro/src/llvm/include/llvm/CodeGen/MachineOperand.h:368:
void llvm::MachineOperand::setIsKill(bool): Assertion `isReg() && !IsDef &&
"Wrong MachineOperand accessor"' failed.

This is due to extra registers pushed and popped are not marked correctly:

>      // Mark the unimportant registers as <def,dead> in the POP.
> -    RegList.push_back(MachineOperand::CreateReg(CurReg, true, false, true));
> +    RegList.push_back(MachineOperand::CreateReg(CurReg, true, false, false, true));

2. It does not check whether the stack location, where an extra reg is pushed
to and popped from, is written by the function.

An example:

$ cat foo.c 
int goo (char*, int, int, int, int);
int hoo (int);

int foo(char* a, int b, int c) {
  int new_move;
  new_move = goo(a, b, c, 1, 0);
  return (hoo(new_move));
}

$ clang -Oz -mthumb -mcpu=cortex-a9 foo.c -S
foo:
@ BB#0:                                 @ %entry
        push.w  {r9, r10, r11, lr}
        movs    r3, #0
>        str     r3, [sp]
        movs    r3, #1
        bl      goo
        bl      hoo
        pop.w   {r9, r10, r11, pc}

$ clang -Oz -mthumb -mcpu=cortex-a9 foo.c -S -mllvm
-enable-SP-update-folding=false
foo:
@ BB#0:                                 @ %entry
        push.w  {r11, lr}
        sub     sp, #8
        movs    r3, #0
>        str     r3, [sp]
        movs    r3, #1
        bl      goo
        bl      hoo
        add     sp, #8
        pop.w   {r11, pc}


With folding SP update into push/pop, R9 is overwritten by foo. If caller of
<foo> has R9 live across the callsite, it will get a corrupted value in R9.


I can think some solutions but none of them looks complete:
1. Add a flag to enabled this feature and disable it by default.
2. Enable this feature only if SP-update is used for stack alignment. For
example:
  push {sl, fp, lr}
  sub sp, #4
becomes
  push {r9, sl, fp, lr}
But
  push {fp, lr}
  sub sp, #8
is kept as is.
This does not prevent the compiler (or compiler writer) from using the extra
space (4-byte in this case) as scratch area on stack.
3. Enable this feature only if there's no write to any address to where extra
registers are pushed . The hard part is to resolve that no store address alias
with such stack slots.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-bugs/attachments/20131128/6c5c1f2a/attachment.html>