[llvm-commits] [llvm] r151877 - in /llvm/trunk: lib/Target/X86/X86ISelLowering.cpp test/CodeGen/X86/sibcall-5.ll

[Chad Rosier]

On Mar 1, 2012, at 8:13 PM, Evan Cheng wrote:

> 
> On Mar 1, 2012, at 6:50 PM, Chad Rosier wrote:
> 
>> 
>> 
>> +  // If anything is glued to the copy, then we can't safely perform a tail call.
>> +  if (Copy->getOpcode() == ISD::CopyToReg &&
>> +      Copy->getNumOperands() == 4)
>> +    return false;
> 
> Hi Chad,
> 
> This is not the right way to check whether anything is glued to the copy. It's checking whether the copytoreg has a 4th argument and the use magic number is fragile. You want to check if the copytoreg has second value and whether that value has any use.

If I understand the comments in SelectionDAG.h correctly, the 4th operand is the glue value.  Thus, this seems to be the correct check.  I do agree, however, that using a magic number is error prone.  Is there a more generic (i.e., accessor function) for checking for glue?

Here's the exact comment and relevant code:
----------
  // This version of the getCopyToReg method takes an extra operand, which
  // indicates that there is potentially an incoming glue value (if Glue is not
  // null) and that there should be a glue result.
  SDValue getCopyToReg(SDValue Chain, DebugLoc dl, unsigned Reg, SDValue N,
                       SDValue Glue) {
    SDVTList VTs = getVTList(MVT::Other, MVT::Glue);
    SDValue Ops[] = { Chain, getRegister(Reg, N.getValueType()), N, Glue };
    return getNode(ISD::CopyToReg, dl, VTs, Ops, Glue.getNode() ? 4 : 3);
  }
----------

> 
> Also since the code fragment before is:
> 
>  if (Copy->getOpcode() != ISD::CopyToReg &&
>      Copy->getOpcode() != ISD::FP_EXTEND)
>    return false;
> 
> You want to write something like
> 
>  if (Copy->getOpcode() != ISD::CopyToReg) {

Don't you mean:

if (Copy->getOpcode() == ISD::CopyToReg) {
  // If anything is glued to the copy...
  if (Glue)
    return false;
}

Specifically, this should be an equals comparison, not a not equal comparison.

 Chad

> 	// If anything is glued to the copy ...
>  } else if (Copy->getOpcode() != ISD::FP_EXTEND)
>    return false;
> 
> Evan
> 
>> +
>>  bool HasRet = false;
>>  for (SDNode::use_iterator UI = Copy->use_begin(), UE = Copy->use_end();
>>       UI != UE; ++UI) {
>> 
>> Modified: llvm/trunk/test/CodeGen/X86/sibcall-5.ll
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/sibcall-5.ll?rev=151877&r1=151876&r2=151877&view=diff
>> ==============================================================================
>> --- llvm/trunk/test/CodeGen/X86/sibcall-5.ll (original)
>> +++ llvm/trunk/test/CodeGen/X86/sibcall-5.ll Thu Mar  1 20:50:46 2012
>> @@ -1,5 +1,6 @@
>> ; RUN: llc < %s -mtriple=i386-apple-darwin8 -mattr=+sse2  | FileCheck %s --check-prefix=X32
>> ; RUN: llc < %s -mtriple=x86_64-apple-darwin -mattr=+sse2 | FileCheck %s --check-prefix=X64
>> +; RUN: llc < %s -mtriple=x86_64-apple-darwin -mattr=-sse3 | FileCheck %s --check-prefix=X64_BAD
>> 
>> ; Sibcall optimization of expanded libcalls.
>> ; rdar://8707777
>> @@ -29,3 +30,31 @@
>> declare float @sinf(float) nounwind readonly
>> 
>> declare double @sin(double) nounwind readonly
>> +
>> +; rdar://10930395
>> +%0 = type opaque
>> +
>> +@"\01L_OBJC_SELECTOR_REFERENCES_2" = external hidden global i8*, section "__DATA, __objc_selrefs, literal_pointers, no_dead_strip"
>> +
>> +define hidden { double, double } @foo2(%0* %self, i8* nocapture %_cmd) uwtable optsize ssp {
>> +; X64_BAD: foo
>> +; X64_BAD: call
>> +; X64_BAD: call
>> +; X64_BAD: call
>> +  %1 = load i8** @"\01L_OBJC_SELECTOR_REFERENCES_2", align 8, !invariant.load !0
>> +  %2 = bitcast %0* %self to i8*
>> +  %3 = tail call { double, double } bitcast (i8* (i8*, i8*, ...)* @objc_msgSend to { double, double } (i8*, i8*)*)(i8* %2, i8* %1) optsize
>> +  %4 = extractvalue { double, double } %3, 0
>> +  %5 = extractvalue { double, double } %3, 1
>> +  %6 = tail call double @floor(double %4) optsize
>> +  %7 = tail call double @floor(double %5) optsize
>> +  %insert.i.i = insertvalue { double, double } undef, double %6, 0
>> +  %insert5.i.i = insertvalue { double, double } %insert.i.i, double %7, 1
>> +  ret { double, double } %insert5.i.i
>> +}
>> +
>> +declare i8* @objc_msgSend(i8*, i8*, ...)
>> +
>> +declare double @floor(double) optsize
>> +
>> +!0 = metadata !{}
>> 
>> 
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>