[ARM] Wrong t2STMIA size optimization in Thumb2SizeReduction

[sdmitrouk]

Summary

Wrong check in Thumb2SizeReduction caused reduction of t2STMIA instruction
to t1STMIA even when t2STMIA has LR register in its register list. This is
wrong as register list of t1STMIA is 8 bit long (for R0-R7 only), thus LR
register (R14) is just thrown away on emitting binary representation of t1STMIA.
It causes some data to be silently lost at run-time, in very rare cases.

[t.p.northover]

Hi Sergey,

The code itself looks fine, but the test can be made much more robust, and simpler at the same time. It's always a good idea to keep some regalloc-bait handy!

 declare i8* @llvm.returnaddress(i32)

 define i32* @foo(i32* %addr, i32 %val0) minsize {
   store i32 %val0, i32* %addr

   %addr1 = getelementptr i32* %addr, i32 1
   %lr = call i8* @llvm.returnaddress(i32 0)
   %lr32 = ptrtoint i8* %lr to i32
   store i32 %lr32, i32* %addr1

   %addr2 = getelementptr i32* %addr1, i32 1
   ret i32* %addr2
}

Cheers.

Tim.

[sdmitrouk]

Replaced test function with much better one provided by Tim.

[sdmitrouk]

Hi Tim,

Thanks a lot for a much better test! I'm not good at writing IR
and tried to reduce output of Clang.

Regards,
Sergey

[t.p.northover]

Hi Sergey,

Thanks for updating the patch, I think this looks fine now.

Cheers.

Tim.

[sdmitrouk]

Tim,

Great, but I don't have commit access. Could you please commit it?

Thanks,
Sergey

[t.p.northover]

Sure, it's r217498.

Cheers.

Tim.

lib/Target/ARM/Thumb2SizeReduction.cpp

Thumb2SizeReduce::VerifyPredAndCC(MachineInstr *MI, const ReduceEntry &Entry,
   return true;
 }
 
 static bool VerifyLowRegs(MachineInstr *MI) {
   unsigned Opc = MI->getOpcode();
   bool isPCOk = (Opc == ARM::t2LDMIA_RET || Opc == ARM::t2LDMIA     ||
                  Opc == ARM::t2LDMDB     || Opc == ARM::t2LDMIA_UPD ||
                  Opc == ARM::t2LDMDB_UPD);
-  bool isLROk = (Opc == ARM::t2STMIA_UPD || Opc == ARM::t2STMDB_UPD);
+  bool isLROk = (Opc == ARM::t2STMDB_UPD);
   bool isSPOk = isPCOk || isLROk;
   for (unsigned i = 0, e = MI->getNumOperands(); i != e; ++i) {
     const MachineOperand &MO = MI->getOperand(i);
     if (!MO.isReg() || MO.isImplicit())
       continue;
     unsigned Reg = MO.getReg();
     if (Reg == 0 || Reg == ARM::CPSR)
       continue;

test/CodeGen/ARM/wrong-t2stmia-size-opt.ll

+; RUN: llc -mcpu=cortex-a9 -O1 -filetype=obj %s -o - | llvm-objdump -arch thumb -mcpu=cortex-a9 -d - | FileCheck %s
+
+target datalayout = "e-m:e-p:32:32-i1:8:32-i8:8:32-i16:16:32-i64:64-v128:64:128-a:0:32-n32-S64"
+target triple = "thumbv7--linux-gnueabi"
+
+declare i8* @llvm.returnaddress(i32)
+
+define i32* @wrong-t2stmia-size-reduction(i32* %addr, i32 %val0) minsize {
+  store i32 %val0, i32* %addr
+  %addr1 = getelementptr i32* %addr, i32 1
+  %lr = call i8* @llvm.returnaddress(i32 0)
+  %lr32 = ptrtoint i8* %lr to i32
+  store i32 %lr32, i32* %addr1
+  %addr2 = getelementptr i32* %addr1, i32 1
+  ret i32* %addr2
+}
+
+; Check that stm writes two registers.  The bug caused one of registers (LR,
+; which invalid for Thumb1 form of STMIA instruction) to be dropped.
+; CHECK: stm{{[^,]*}}, {{{.*,.*}}}