[llvm-commits] [llvm] r130205 - in /llvm/trunk: lib/CodeGen/SelectionDAG/FastISel.cpp test/CodeGen/X86/fast-isel.ll

[Nick Lewycky]

On 30 September 2011 19:19, Nick Lewycky <nlewycky at google.com> wrote:

> On 26 April 2011 10:18, Dan Gohman <gohman at apple.com> wrote:
>
>> Author: djg
>> Date: Tue Apr 26 12:18:34 2011
>> New Revision: 130205
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=130205&view=rev
>> Log:
>> Fast-isel support for simple inline asms.
>>
>> Modified:
>>    llvm/trunk/lib/CodeGen/SelectionDAG/FastISel.cpp
>>    llvm/trunk/test/CodeGen/X86/fast-isel.ll
>>
>> Modified: llvm/trunk/lib/CodeGen/SelectionDAG/FastISel.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/FastISel.cpp?rev=130205&r1=130204&r2=130205&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/lib/CodeGen/SelectionDAG/FastISel.cpp (original)
>> +++ llvm/trunk/lib/CodeGen/SelectionDAG/FastISel.cpp Tue Apr 26 12:18:34
>> 2011
>> @@ -482,14 +482,35 @@
>>  }
>>
>>  bool FastISel::SelectCall(const User *I) {
>> -  const Function *F = cast<CallInst>(I)->getCalledFunction();
>> +  const CallInst *Call = cast<CallInst>(I);
>> +
>> +  // Handle simple inline asms.
>> +  if (const InlineAsm *IA = dyn_cast<InlineAsm>(Call->getArgOperand(0)))
>> {
>>
>
> Are you sure this is what you mean? This is checking that the first
> argument to a call is the result of an inline asm run earlier. Did you mean
> "if (const InlineAsm *IA = dyn_cast<InlineAsm>(Call->getCalledValue())) {" ?
>

Ping! I still think this code is wrong.

If so, make sure to hoist the "const Function *F =
> Call->getCalledFunction();" stuff below above this and reuse it.
>

This comment however was wrong. The patch to fix it is:

Index: lib/CodeGen/SelectionDAG/FastISel.cpp
===================================================================
--- lib/CodeGen/SelectionDAG/FastISel.cpp       (revision 141714)
+++ lib/CodeGen/SelectionDAG/FastISel.cpp       (working copy)
@@ -494,7 +494,7 @@
   const CallInst *Call = cast<CallInst>(I);

   // Handle simple inline asms.
-  if (const InlineAsm *IA = dyn_cast<InlineAsm>(Call->getArgOperand(0))) {
+  if (const InlineAsm *IA = dyn_cast<InlineAsm>(Call->getCalledValue())) {
     // Don't attempt to handle constraints.
     if (!IA->getConstraintString().empty())
       return false;

Nick

 +    // Don't attempt to handle constraints.
>> +    if (!IA->getConstraintString().empty())
>> +      return false;
>> +
>> +    unsigned ExtraInfo = 0;
>> +    if (IA->hasSideEffects())
>> +      ExtraInfo |= InlineAsm::Extra_HasSideEffects;
>> +    if (IA->isAlignStack())
>> +      ExtraInfo |= InlineAsm::Extra_IsAlignStack;
>> +
>> +    BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, DL,
>> +            TII.get(TargetOpcode::INLINEASM))
>> +      .addExternalSymbol(IA->getAsmString().c_str())
>> +      .addImm(ExtraInfo);
>> +    return true;
>> +  }
>> +
>> +  const Function *F = Call->getCalledFunction();
>>   if (!F) return false;
>>
>>   // Handle selected intrinsic function calls.
>>   switch (F->getIntrinsicID()) {
>>   default: break;
>>   case Intrinsic::dbg_declare: {
>> -    const DbgDeclareInst *DI = cast<DbgDeclareInst>(I);
>> +    const DbgDeclareInst *DI = cast<DbgDeclareInst>(Call);
>>     if (!DIVariable(DI->getVariable()).Verify() ||
>>         !FuncInfo.MF->getMMI().hasDebugInfo())
>>       return true;
>> @@ -521,7 +542,7 @@
>>   }
>>   case Intrinsic::dbg_value: {
>>     // This form of DBG_VALUE is target-independent.
>> -    const DbgValueInst *DI = cast<DbgValueInst>(I);
>> +    const DbgValueInst *DI = cast<DbgValueInst>(Call);
>>     const TargetInstrDesc &II = TII.get(TargetOpcode::DBG_VALUE);
>>     const Value *V = DI->getValue();
>>     if (!V) {
>> @@ -550,7 +571,7 @@
>>     return true;
>>   }
>>   case Intrinsic::eh_exception: {
>> -    EVT VT = TLI.getValueType(I->getType());
>> +    EVT VT = TLI.getValueType(Call->getType());
>>     if (TLI.getOperationAction(ISD::EXCEPTIONADDR,
>> VT)!=TargetLowering::Expand)
>>       break;
>>
>> @@ -561,18 +582,18 @@
>>     unsigned ResultReg = createResultReg(RC);
>>     BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, DL,
>> TII.get(TargetOpcode::COPY),
>>             ResultReg).addReg(Reg);
>> -    UpdateValueMap(I, ResultReg);
>> +    UpdateValueMap(Call, ResultReg);
>>     return true;
>>   }
>>   case Intrinsic::eh_selector: {
>> -    EVT VT = TLI.getValueType(I->getType());
>> +    EVT VT = TLI.getValueType(Call->getType());
>>     if (TLI.getOperationAction(ISD::EHSELECTION, VT) !=
>> TargetLowering::Expand)
>>       break;
>>     if (FuncInfo.MBB->isLandingPad())
>> -      AddCatchInfo(*cast<CallInst>(I), &FuncInfo.MF->getMMI(),
>> FuncInfo.MBB);
>> +      AddCatchInfo(*Call, &FuncInfo.MF->getMMI(), FuncInfo.MBB);
>>     else {
>>  #ifndef NDEBUG
>> -      FuncInfo.CatchInfoLost.insert(cast<CallInst>(I));
>> +      FuncInfo.CatchInfoLost.insert(Call);
>>  #endif
>>       // FIXME: Mark exception selector register as live in.  Hack for
>> PR1508.
>>       unsigned Reg = TLI.getExceptionSelectorRegister();
>> @@ -586,7 +607,7 @@
>>     BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, DL,
>> TII.get(TargetOpcode::COPY),
>>             ResultReg).addReg(Reg);
>>
>> -    bool ResultRegIsKill = hasTrivialKill(I);
>> +    bool ResultRegIsKill = hasTrivialKill(Call);
>>
>>     // Cast the register to the type of the selector.
>>     if (SrcVT.bitsGT(MVT::i32))
>> @@ -599,7 +620,7 @@
>>       // Unhandled operand. Halt "fast" selection and bail.
>>       return false;
>>
>> -    UpdateValueMap(I, ResultReg);
>> +    UpdateValueMap(Call, ResultReg);
>>
>>     return true;
>>   }
>>
>> Modified: llvm/trunk/test/CodeGen/X86/fast-isel.ll
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/fast-isel.ll?rev=130205&r1=130204&r2=130205&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/test/CodeGen/X86/fast-isel.ll (original)
>> +++ llvm/trunk/test/CodeGen/X86/fast-isel.ll Tue Apr 26 12:18:34 2011
>> @@ -20,6 +20,7 @@
>>   %t6 = add i32 %t5, 2
>>   %t7 = getelementptr i32* %y, i32 1
>>   %t8 = getelementptr i32* %t7, i32 %t6
>> +  call void asm sideeffect "hello world", ""()
>>   br label %exit
>>
>>  exit:
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20111011/1457ee69/attachment.html>