[PATCH] ARM and Thumb Segmented Stacks

Tim Northover t.p.northover at gmail.com
Thu Feb 27 04:12:52 PST 2014


Hi Alex,

Thanks for the updated patch.

> It looks like bionic favors using a coprocessor, which sounds in line
> with what you're saying LLVM does. I have removed the check at the
> special address and left it to only check the coprocessor now.

There still seems to be a separate code path for Thumb here, involving
a special STACK_LIMIT symbol by the looks of it. Thumb's got MRC
instructions too, so you should probably be using those.

> On x86 at least the __morestack function is invoked with an unaligned
> stack. I wouldn't consider myself an expert on this function, but all
> examples I've seen write __morestack in assembly, so I don't think
> it's meant to be written in something like C (as in, I don't think it
> expects an aligned stack).

But doesn't it then jump back to this callee in some weird way? Unless
it specifically realigns the stack, the rest of the program is going
to be non-conformant.

Onto the actual code:

+    // add SR0, SR0, offset*4
+    // AddDefaultPred(BuildMI(GetMBB, DL, TII.get(ARM::ADDri), ScratchReg0)
+    //                .addReg(ScratchReg0).addImm(4*TlsOffset)).addReg(0);

This is commented out and can be removed.

+    AddDefaultPred(BuildMI(AllocMBB, DL, TII.get(ARM::tPOP)))
+      .addReg(ScratchReg0);
+
+    // mov lr, SR0
+    AddDefaultPred(BuildMI(AllocMBB, DL, TII.get(ARM::tMOVr), ARM::LR)
+                   .addReg(ScratchReg0));

If you use t2LDMIA_UPD here you save an instruction (LR can be used
directly) and make the sequence more similar between ARM & Thumb.

+    AddDefaultPred(BuildMI(AllocMBB, DL, TII.get(ARM::tMOVr), ARM::PC)
+                   .addReg(ARM::LR));

These need to be "bx lr" instructions otherwise Thumb-interworking
will be broken: "mov pc, lr" doesn't switch to/from thumb mode if
needed. An alternative would be to fold it into the push/pop
instructions, though that would mean always pushing 4 regs (3 + 1 for
alignment) since the last one has to be lr for that to work. Something
like:

    push {r4, r5, r11, lr}
    [...]
    pop {r4, r5, r11, pc}
    [...]
    pop {r4, r5, r11, lr}

You'd save on code size, but it would probably be slower in many
situations (I've not tested).

Cheers.

Tim.



More information about the llvm-commits mailing list