[PATCH] D63677: [ARM] Don't reserve R12 on Thumb1 as an emergency spill slot.

[Oliver Stannard (Linaro) via Phabricator via llvm-commits]

ostannard added a comment.

This looks a lot like a bug I hit a while ago while writing a fuzzer for ABI
compatibility between clang and gcc. I've currently got Thumb1-only targets
disabled because of this. Applying this patch doesn't get the fuzzer fully
passing, but it does drop the failure rate significantly, and I expect there
are more bugs to find once this one is fixed. I should have time later this
week to reduce some of the remaining failures, but since this test suite has
never passed for Thumb1 I don't think it should block this patch.



================
Comment at: lib/Target/ARM/ARMFrameLowering.cpp:1973
         assert(!MRI.isReserved(Reg) && "Should not be reserved");
-        if (!MRI.isPhysRegUsed(Reg))
+        if (Reg != ARM::LR && !MRI.isPhysRegUsed(Reg))
           ExtraCSSpill = true;
----------------
Could you expand the comment at the definition of ExtraCSSpill to explain the conditions in which it will be set?


================
Comment at: test/CodeGen/Thumb/emergency-spill-slot.ll:334
+; so we don't generate code that requires an emergency spill slot we never
+; allocated.  If the store gets eliminated, this testcase probably needs
+; to be rewritten.)
----------------
Could we just not have any CHECK lines for this test, if we don't care about the output, and are just making sure we don't crash?


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63677/new/

https://reviews.llvm.org/D63677