[PATCH] D31962: DAG: Set hasCalls on frame info earlier

Justin Bogner via llvm-commits llvm-commits at lists.llvm.org
Mon Jun 19 14:54:15 PDT 2017


Matt Arsenault <arsenm2 at gmail.com> writes:
>> On Jun 19, 2017, at 14:23, Justin Bogner via llvm-commits
>> <llvm-commits at lists.llvm.org> wrote:
>> 
>> Matt Arsenault via Phabricator <reviews at reviews.llvm.org
>> <mailto:reviews at reviews.llvm.org>> writes:
>>> 
>>> +bool SITargetLowering::shouldAssumeEmittedAsCall(ImmutableCallSite
>>> CS) const {
>>> +  const Function *F = CS.getCalledFunction();
>>> +  return !F || !F->isIntrinsic();
>> 
>> When does the !F case come up?
>
> Indirect calls (also I think some cases where the function is bitcast
> to a different type).

Makes sense.

>>> +}
>>> +
>>> bool SITargetLowering::isShuffleMaskLegal(const SmallVectorImpl<int> &,
>>>                                           EVT) const {
>>>   // SI has some legal vector types, but no legal vector operations. Say no
>>> Index: lib/CodeGen/SelectionDAG/FunctionLoweringInfo.cpp
>>> ===================================================================
>>> --- lib/CodeGen/SelectionDAG/FunctionLoweringInfo.cpp
>>> +++ lib/CodeGen/SelectionDAG/FunctionLoweringInfo.cpp
>>> @@ -190,6 +190,8 @@
>>>                 MF->getFrameInfo().setHasOpaqueSPAdjustment(true);
>>>             }
>>>           }
>>> +        } else if (TLI->shouldAssumeEmittedAsCall(CS)) {
>>> +          MF->getFrameInfo().setHasCalls(true);
>>>         }
>>>       }
>>> 
>>> Index: include/llvm/Target/TargetLowering.h
>>> ===================================================================
>>> --- include/llvm/Target/TargetLowering.h
>>> +++ include/llvm/Target/TargetLowering.h
>>> @@ -2811,6 +2811,12 @@
>>>     return false;
>>>   }
>>> 
>>> +  /// Return true if it should be assumed this will be emitted as a call, not a
>>> +  /// tail call or special instruction.
>>> +  virtual bool shouldAssumeEmittedAsCall(ImmutableCallSite CS) const {
>>> +    return false;
>>> +  }
>> 
>> Based on the comment, defaulting to returning false seems a bit funny
>> (won't most of these be emitted as a call?).
>
> That would nice, but this is to fix a bunch of random backend test
> failures. This whole thing is kind of problematic because it involves
> predicting whether or not the call will be a tail call or not. With
> the more correct conservative answer many tests fail, mostly due to
> emitting tail calls which don’t count or special lib calls replaced
> with instructions.

Fair enough. Maybe you could reword the doc comment to make it clear
that this conservatively returns false when it isn't sure?

Anyways, this LGTM. Feel free to commit.


More information about the llvm-commits mailing list