<html><head><meta http-equiv="Content-Type" content="text/html charset=iso-8859-1"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;">The code has changed a lot over the years. Looks like at some point of time the assumption was broken. calculateCallsInformation() may have eliminated the pseudo set up instructions already.<div><br></div><div><div style="margin: 0px; font-size: 11px; font-family: Menlo; color: rgb(215, 57, 30);"><span style="color: #000000">    </span>// If call frames are not being included as part of the stack frame, and                                                                                                                                                                       </div><div style="margin: 0px; font-size: 11px; font-family: Menlo; color: rgb(215, 57, 30);"><span style="color: #000000">    </span>// the target doesn't indicate otherwise, remove the call frame pseudos                                                                                                                                                                        </div><div style="margin: 0px; font-size: 11px; font-family: Menlo; color: rgb(215, 57, 30);"><span style="color: #000000">    </span>// here. The sub/add sp instruction pairs are still inserted, but we don't                                                                                                                                                                     </div><div style="margin: 0px; font-size: 11px; font-family: Menlo; color: rgb(215, 57, 30);"><span style="color: #000000">    </span>// need to track the SP adjustment for frame index elimination.                                                                                                                                                                                </div><div style="margin: 0px; font-size: 11px; font-family: Menlo;">    <span style="color: #d03cff">if</span> (TFI->canSimplifyCallFramePseudos(Fn))</div><div style="margin: 0px; font-size: 11px; font-family: Menlo;">=>    TFI->eliminateCallFramePseudoInstr(Fn, *I->getParent(), I);</div><div style="margin: 0px; font-size: 11px; font-family: Menlo;"><br></div><div style="margin: 0px; font-size: 11px; font-family: Menlo;">Perhaps there is a bug in canSimplifyCallFramePseudos?</div><div style="margin: 0px; font-size: 11px; font-family: Menlo;"><br></div><div style="margin: 0px; font-size: 11px; font-family: Menlo;">Evan</div><div style="margin: 0px; font-size: 11px; font-family: Menlo;"><br></div><div><div>On Sep 26, 2013, at 12:00 PM, Krzysztof Parzyszek <<a href="mailto:kparzysz@codeaurora.org">kparzysz@codeaurora.org</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite">Consider this example:<br><br>--- ex.ll ---<br>declare void @bar()<br><br>; Function Attrs: nounwind optsize<br>define void @main() {<br>entry:<br>  %hin = alloca [256 x i32], align 4<br>  %xin = alloca [256 x i32], align 4<br>  call void @bar()<br>  ret void<br>}<br>-------------<br><br><br>Freshly built llc:<br><br>llc -O2 -march=x86 < ex.ll -print-before-all<br><br># *** IR Dump Before Prologue/Epilogue Insertion & Frame Finalization ***:<br># Machine code for function main: Post SSA<br>Frame Objects:<br>  fi#0: size=1024, align=4, at location [SP+4]<br>  fi#1: size=1024, align=4, at location [SP+4]<br><br>BB#0: derived from LLVM BB %entry<br>        ADJCALLSTACKDOWN32 0, %ESP<imp-def>, %EFLAGS<imp-def,dead>, %ESP<imp-use><br>        CALLpcrel32 <ga:@bar>, <regmask>, %ESP<imp-use>, %ESP<imp-def><br>        ADJCALLSTACKUP32 0, 0, %ESP<imp-def>, %EFLAGS<imp-def,dead>, %ESP<imp-use><br>        RET<br><br># End machine code for function main.<br><br>before replace frame indices<br># Machine code for function main: Post SSA<br>Frame Objects:<br>  fi#0: size=1024, align=4, at location [SP-1024]<br>  fi#1: size=1024, align=4, at location [SP-2048]<br><br>BB#0: derived from LLVM BB %entry<br>        %ESP<def,tied1> = SUB32ri %ESP<tied0>, 2060, %EFLAGS<imp-def,dead>; flags: FrameSetup<br>        PROLOG_LABEL <MCSym=.Ltmp0><br>        CALLpcrel32 <ga:@bar>, <regmask>, %ESP<imp-use>, %ESP<imp-def><br>        %ESP<def,tied1> = ADD32ri %ESP<tied0>, 2060, %EFLAGS<imp-def,dead><br>        RET<br><br># End machine code for function main.<br><br><br><br>Let's see what happens if we remove the call to "bar".<br><br>There aren't any pseudocodes that set up the frame to begin with, even though the SP is actually modified.  (This is to show that RS has no way of finding out that SP was actually adjusted in such cases.)<br><br><br># *** IR Dump Before Prologue/Epilogue Insertion & Frame Finalization ***:<br># Machine code for function main: Post SSA<br>Frame Objects:<br>  fi#0: size=1024, align=4, at location [SP+4]<br>  fi#1: size=1024, align=4, at location [SP+4]<br><br>BB#0: derived from LLVM BB %entry<br>        RET<br><br># End machine code for function main.<br><br>before replace frame indices<br># Machine code for function main: Post SSA<br>Frame Objects:<br>  fi#0: size=1024, align=4, at location [SP-1024]<br>  fi#1: size=1024, align=4, at location [SP-2048]<br><br>BB#0: derived from LLVM BB %entry<br>        %ESP<def,tied1> = SUB32ri %ESP<tied0>, 2048, %EFLAGS<imp-def,dead>; flags: FrameSetup<br>        PROLOG_LABEL <MCSym=.Ltmp0><br>        %ESP<def,tied1> = ADD32ri %ESP<tied0>, 2048, %EFLAGS<imp-def,dead><br>        RET<br><br># End machine code for function main.<br><br><br><br>And here's where the problem becomes more apparent.<br><br>Compile for Thumb and see that there is a virtual register used in the frame setup:<br><br># *** IR Dump Before Prologue/Epilogue Insertion & Frame Finalization ***:<br># Machine code for function main: Post SSA<br>Frame Objects:<br>  fi#0: size=1024, align=4, at location [SP]<br>  fi#1: size=1024, align=4, at location [SP]<br><br>BB#0: derived from LLVM BB %entry<br>        tBX_RET pred:14, pred:%noreg<br><br># End machine code for function main.<br><br>before replace frame indices<br># Machine code for function main: Post SSA<br>Frame Objects:<br>  fi#0: size=1024, align=4, at location [SP-1032]<br>  fi#1: size=1024, align=4, at location [SP-2056]<br>  fi#2: size=4, align=4, at location [SP-4]<br>  fi#3: size=4, align=4, at location [SP-8]<br>Constant Pool:<br>  cp#0: -2048, align=4<br>  cp#1: 2048, align=4<br><br>BB#0: derived from LLVM BB %entry<br>    Live Ins: %R4 %LR<br>        tPUSH pred:14, pred:%noreg, %R4<kill>, %LR<kill>, %SP<imp-def>, %SP<imp-use>; flags: FrameSetup<br>        %vreg0<def> = tLDRpci <cp#0>, pred:14, pred:%noreg; flags: FrameSetup tGPR:%vreg0<br>        %SP<def,tied1> = tADDhirr %SP<tied0>, %vreg0<kill>, pred:14, pred:%noreg; tGPR:%vreg0<br>        %vreg1<def> = tLDRpci <cp#1>, pred:14, pred:%noreg; tGPR:%vreg1<br>        %SP<def,tied1> = tADDhirr %SP<tied0>, %vreg1<kill>, pred:14, pred:%noreg; tGPR:%vreg1<br>        tPOP_RET pred:14, pred:%noreg, %R4<def>, %PC<def>, %SP<imp-def>, %SP<imp-use><br><br># End machine code for function main.<br><br><br>On Thumb you can save/restore a register without having to use a spill slot, so the scavenger won't run into problems, but if a target had to spill, we would end up with a register save before the SP update, and restore after the SP update, and the RS would use the same offset in both instructions.<br>I don't have a working testcase (i.e. one that demonstrates the failure) that I can post, but if I cheat the RS into believing that it has to spill, the problem will happen.<br><br>Here's a sample result of this.  Don't mind the FixedStack-1, I explicitly used a base offset of 0 in the code, and this was to illustrate the lack of adjustment in RS:<br><br>        tSTRspi %R1<kill>, %SP, 0, pred:14, pred:%noreg; mem:ST4[FixedStack-1]    <- spill to *(SP+0)<br>        %R1<def> = tLDRpci <cp#1>, pred:14, pred:%noreg<br>        %SP<def,tied1> = tADDhirr %SP<tied0>, %R1<kill>, pred:14, pred:%noreg     <- SP = something different<br>        %R3<def> = tLDRspi %SP, 0, pred:14, pred:%noreg; mem:LD4[FixedStack-1]<br>        %R1<def> = tLDRspi %SP, 0, pred:14, pred:%noreg; mem:LD4[FixedStack-1]    <- restore from *(NewSP+0)   !!<br><br><br>-Krzysztof<br><br><br><br>On 9/26/2013 1:24 PM, Evan Cheng wrote:<br><blockquote type="cite">CallFrameSetupOpcode is a pseudo opcode like X86::ADJCALLSTACKDOWN64.<br>That means when the code is expected to be called before the pseudo<br>instructions are eliminated. I don't know why it's not the case for you.<br>A quick look at PEI code indicates the pseudo's should not have been<br>removed at the time when replaceFrameIndices are run.<br><br>Evan<br><br><br>On Sep 25, 2013, at 8:57 AM, Krzysztof Parzyszek<br><<a href="mailto:kparzysz@codeaurora.org">kparzysz@codeaurora.org</a> <<a href="mailto:kparzysz@codeaurora.org">mailto:kparzysz@codeaurora.org</a>>> wrote:<br><br><blockquote type="cite">Hi All,<br>I'm dealing with a problem where the spill/restore instructions<br>inserted during scavenging span an adjustment of the SP/FP register.<br> The result is that despite the base register (SP/FP) being changed<br>between the spill and the restore, both store and load use the same<br>immediate offset.<br><br>I see code in the PEI (replaceFrameIndices) that is supposed to track<br>the SP/FP adjustment:<br><br>----------------------------------------<br>void PEI::replaceFrameIndices(MachineBasicBlock *BB,<br>                             MachineFunction &Fn, int &SPAdj) {<br> const TargetMachine &TM = Fn.getTarget();<br> assert(TM.getRegisterInfo() &&<br>        "TM::getRegisterInfo() must be implemented!");<br> const TargetInstrInfo &TII = *Fn.getTarget().getInstrInfo();<br> const TargetRegisterInfo &TRI = *TM.getRegisterInfo();<br> const TargetFrameLowering *TFI = TM.getFrameLowering();<br> bool StackGrowsDown =<br>   TFI->getStackGrowthDirection() ==<br>               TargetFrameLowering::StackGrowsDown;<br> int FrameSetupOpcode   = TII.getCallFrameSetupOpcode();<br> int FrameDestroyOpcode = TII.getCallFrameDestroyOpcode();<br><br> if (RS && !FrameIndexVirtualScavenging) RS->enterBasicBlock(BB);<br><br> for (MachineBasicBlock::iterator I = BB->begin(); I != BB->end(); ) {<br><br>   if (I->getOpcode() == FrameSetupOpcode ||<br>       I->getOpcode() == FrameDestroyOpcode) {<br>     // Remember how much SP has been adjusted to create the call<br>     // frame.<br>     int Size = I->getOperand(0).getImm();<br><br>     if ((!StackGrowsDown && I->getOpcode() == FrameSetupOpcode) ||<br>         (StackGrowsDown && I->getOpcode() == FrameDestroyOpcode))<br>       Size = -Size;<br><br>     SPAdj += Size;<br><br> [...]<br>----------------------------------------<br><br><br>The problem is that it expects frame-setup and frame-destroy opcodes,<br>but at the time it runs (after emitPrologue/emitEpilogue) the frame<br>setup and teardown will be expanded into instruction sequences that<br>can be different for each target, let alone having the immediate value<br>in the 0-th operand.<br><br>As I see, this code won't work, although I'm not sure what was the<br>original idea behind it.  Should this code run before the target<br>specific generation of prolog/epilog?  Even then, there won't need to<br>be ADJCALLSTACKUP/DOWN instructions (if it's a leaf function).  If it<br>runs where it should, should it instead use some target-specific hook<br>that identifies the actual stack adjustment amount?<br><br>-Krzysztof<br><br><br>--<br>Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,<br>hosted by The Linux Foundation<br>_______________________________________________<br>LLVM Developers mailing list<br><a href="mailto:LLVMdev@cs.uiuc.edu">LLVMdev@cs.uiuc.edu</a><br><mailto:LLVMdev@cs.uiuc.edu>http://llvm.cs.uiuc.edu<br><http://llvm.cs.uiuc.edu/><br>http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev<br></blockquote><br></blockquote><br><br>-- <br>Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation<br></blockquote></div><br></div></body></html>