X86 Calls marked to clobber RSP. Intended?

[Jakob Stoklund Olesen]

On Aug 13, 2014, at 11:13 PM, Frédéric Riss <friss at apple.com> wrote:

> Hi Jakob,
> 
> You seem to have introduced the RegMask handling in MachineInstr::setPhysRegsDeadExcept() 2 years ago (r149715). Can you tell me if the behaviour described bellow is really on purpose, or if the %rsp clobber is an unneeded side-effect. If the latter, do you see any way to prevent it?

Hi Frédéric,

As Tim pointed out, some calling conventions do actually modify the stack pointer. But even when they don't, it is normal to have instructions around a call that do modify the stack pointer to set up a call frame.

The PEI pass knows where to find stack variables even when the stack pointer is moving, and it seems to me that debug value tracking should do the same. We also have two stack slot coloring passes that know exactly where stack variables are live.

Whether calls actually clobber %rsp or not, using those clobbers for debug info range computations for stack variables seems very fragile.

Thanks,
/jakob


> 
>> On 13 Aug 2014, at 13:01, Frédéric Riss <friss at apple.com> wrote:
>> 
>> Hi,
>> 
>> The debug info code and more specifically the code in 
>> llvm/lib/CodeGen/AsmPrinter/DbgValueHistoryCalculator.cpp 
>> tries to infer debug variables live ranges using the MachineInstr operand clobbers. Under some circumstances it fails to prove that spilled variables are live during the whole function because those variables location is described in terms of stack pointer+offset but the call MIs are marked to clobber the stack pointer. Which IMHO doesn’t make much sense.
>> 
>> I tested this on X86, don’t know if other backends are affected.
>> 
>> The code that marks RSP as clobbered is in 
>> lib/CodeGen/MachineInstr.cpp:void MachineInstr::setPhysRegsDeadExcept():
>> 
>> The X86 calls have a RegMask argument that trigger this piece of code in the above function:
>> // This is a call with a register mask operand.
>> // Mask clobbers are always dead, so add defs for the non-dead defines.
>> if (HasRegMask)
>>   for (ArrayRef<unsigned>::iterator I = UsedRegs.begin(), E = UsedRegs.end();
>>        I != E; ++I)
>>     addRegisterDefined(*I, &TRI);
>> 
>> The call instructions have an implicit RSP use which is turned into an implicit def by the above piece of code.
>> 
>> This doesn’t happen every time as calls emitted by FastISel don’t have the aforementioned clobber added to them (Looks like FastISel doesn’t add the implicit uses before calling setPhysRegsDeadExcept).
>> 
>> For this simple code:
>> 
>> int bar();
>> int foo() {
>> 	return bar();
>> }
>> 
>> if compiled without options I get:
>> 
>> 	CALL64pcrel32 <ga:@bar>, <regmask>, %RSP<imp-use>, %AL<imp-use,kill>, %EAX<imp-def>
>> 
>> When using -mllvm -fast-isel=0 I get:
>> 
>> 	CALL64pcrel32 <ga:@bar>, <regmask>, %RSP<imp-use>, %AL<imp-use,kill>, %RSP<imp-def>, %EAX<imp-def>
>> 
>> Any thoughts? Does the RSP clobber make sense? Could it be removed? I’d happily work on a patch, if I could figure out the exact expected semantics of the above code...
>> 
>> Thanks!
>> Fred 
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>