jcai19 marked 4 inline comments as done. jcai19 added inline comments.
================ Comment at: llvm/lib/Target/ARM/ARMExpandPseudoInsts.cpp:1927 + .add(predOps(ARMCC::AL)) + .addReg(ARM::LR); + ---------------- efriedma wrote: > I think you need to ensure that lr actually contains the correct value, > somehow. Normally the call will come before anything that would clobber lr, > but you're not actually enforcing that anywhere: LR isn't listed as an input > to BL_PUSHLR. > > To make this work correctly, I think the return address actually needs to be > an argument to the BL_PUSHLR instruction. See > ARMTargetLowering::LowerRETURNADDR for how to make an appropriate copy. My takeaway from your comment is to mark LR explicitly alive to make sure compiler will restore LR if it clobbers the register before the BL_PUSHLR instruction. Did I understand correctly? Thanks. Anyway, this change seems to be needed once I turned on -verify-machineinstrs in the unit test, which complained the push/stmdb instruction trying to use a dead LR register. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65019/new/ https://reviews.llvm.org/D65019 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
