[llvm-commits] [llvm] r149597 - in /llvm/trunk: lib/Target/X86/X86ISelLowering.cpp test/CodeGen/X86/uint64-to-float.ll

[Evan Cheng]

On Feb 2, 2012, at 11:07 AM, Jakob Stoklund Olesen wrote:

> 
> On Feb 1, 2012, at 11:48 PM, Lang Hames wrote:
> 
>> Author: lhames
>> Date: Thu Feb  2 01:48:37 2012
>> New Revision: 149597
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=149597&view=rev
>> Log:
>> Set EFLAGS correctly in EmitLoweredSelect on X86.
>> 
>> Modified:
>>   llvm/trunk/lib/Target/X86/X86ISelLowering.cpp
>>   llvm/trunk/test/CodeGen/X86/uint64-to-float.ll
>> 
>> +// Check whether the given instruction should have had a kill marker on
>> +// the EFLAGS operand.
>> +static bool shouldHaveEFlagsKill(MachineBasicBlock::iterator SelectItr,
> 
> I think the function name should reflect that you are actually recomputing and changing the kill flag.
> 
>> +    if (mi.definesRegister(X86::EFLAGS)) {
>> +      // Should have kill-flag - update below.
>> +      break;
>> +    }
> 
> Technically, you want modifiesRegister() here, although there is no difference when EFLAGS has no aliases.
> 
>> +  // We found a def, or hit the end of the basic block. SelectMI should have a
>> +  // kill flag on EFLAGS.
> 
> Not so fast! What if EFLAGS is live out of the block?

I don't think that's possible at isel time, right?

Evan

> 
>> +  MachineInstr& SelectMI = *SelectItr;
>> +  MachineOperand* EFlagsOp = SelectMI.findRegisterUseOperand(X86::EFLAGS);
>> +  assert(EFlagsOp != 0 && "No EFLAGS operand on select instruction?");
>> +  EFlagsOp->setIsKill();
> 
> Just use MI->addRegisterKilled() here.
> 
>> +  return true;
>> +}
>> +
>> MachineBasicBlock *
>> X86TargetLowering::EmitLoweredSelect(MachineInstr *MI,
>>                                     MachineBasicBlock *BB) const {
>> @@ -12089,8 +12114,10 @@
>>  // If the EFLAGS register isn't dead in the terminator, then claim that it's
>>  // live into the sink and copy blocks.
>>  if (!MI->killsRegister(X86::EFLAGS)) {
>> -    copy0MBB->addLiveIn(X86::EFLAGS);
>> -    sinkMBB->addLiveIn(X86::EFLAGS);
>> +    if (!shouldHaveEFlagsKill(MI, BB)) {
>> +      copy0MBB->addLiveIn(X86::EFLAGS);
>> +      sinkMBB->addLiveIn(X86::EFLAGS);
>> +    }
> 
> &&?
> 
>> --- llvm/trunk/test/CodeGen/X86/uint64-to-float.ll (original)
>> +++ llvm/trunk/test/CodeGen/X86/uint64-to-float.ll Thu Feb  2 01:48:37 2012
>> @@ -6,37 +6,12 @@
>> target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32:64"
>> target triple = "x86_64-apple-darwin10.0.0"
>> 
>> -; FIXME: This test could generate this code:
> 
> Yay!
> 
> /jakob
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits