[IR] Add opt-in flag to isIndirectCall() to consider inlineasm

[madhur13490]

Summary

Inline asm is a black-box to compilers and
many compilers don't analyze the statement
to draw any conclusion. Typically the asm
statement can be an arbitrary piece of code
which could have an indirect call. The current
function doesn't behave so as it returns
false when inlineasm is present.

This patch adds a flag to isIndirectCall()
which would allow analyses to consider that
inline asm may have an indirect call. Default value
of this flag is false to retain the current behaviour.

[rampitec]

It shall probably be target dependent.

llvm/lib/IR/Instructions.cpp

289

It will give wrong answer for non-asm. "return !isInlineAsm() || !InlineAsmMayHaveIndirectCall;"

[madhur13490]

In D98780#2632418, @rampitec wrote:

It shall probably be target dependent.

Well, it is useful for core analyses too. Targets can choose to pass the flag as their need.

llvm/lib/IR/Instructions.cpp

289

I don't think why it would return wrong answer. Here is the truth table of the operation:

1. InlineAsmMayHaveIndirectCall = false, isInlineAsm() = false --> return "true" (as expected)
2. InlineAsmMayHaveIndirectCall = false, isInlineAsm() = true --> return "false" (as expected)
3. InlineAsmMayHaveIndirectCall = true, isInlineAsm() = true --> return "true" (as expected)
4. InlineAsmMayHaveIndirectCall = true, isInlineAsm() = false --> return "false" (as expected)

1 & 2 above are according to today's behavior. 3 & 4 are the new ones. Which of the above is incorrect?

FWIW, "==" operator enacts XNOR for bools.

[rampitec]

In D98780#2632496, @madhur13490 wrote:

In D98780#2632418, @rampitec wrote:

It shall probably be target dependent.

Well, it is useful for core analyses too. Targets can choose to pass the flag as their need.

Ah, right, default is false which preserves current behavior. This is OK.

llvm/lib/IR/Instructions.cpp

289

Last case is incorrect. If it is not isInlineAsm() we should return true, just like now. It does not matter what inline asm may or may not contain then.

But my condition is also wrong. I think it is "return !isInlineAsm() || InlineAsmMayHaveIndirectCall;".

[arsenm]

I don't think this should be done. The point of the function is to check if the IR instruction is an indirect call. inline is a different entity

[madhur13490]

In D98780#2632539, @arsenm wrote:

I don't think this should be done. The point of the function is to check if the IR instruction is an indirect call. inline is a different entity

But in that case the current function should not even do anything related to inlineAsm(). Treating the presence of inlineAsm is different for difference clients and that is what the flag offers.

llvm/lib/IR/Instructions.cpp

289

No, if you're passing inlineAsmMayHaveIndirectCall = true then you're considering that the statement might have one inside the block box.

So, it essentially means "isInlineAsm() == presence of indirect call". If inlineAsm() = true then you have indirect call (condition 3). If inlineAsm() = false then you don't have indirect call (condition 4).

The function should either choose to NOT consider anything about inlineAsm() or should provide flexibility to consider. In latter case, it is not useful and we have to check !callee everywhere.

[madhur13490]

llvm/lib/IR/Instructions.cpp

289

No, if you're passing inlineAsmMayHaveIndirectCall = true then you're considering that the statement might have one inside the block box.

So, it essentially means "isInlineAsm() == presence of indirect call". If inlineAsm() = true then you have indirect call (condition 3). If inlineAsm() = false then you don't have indirect call (condition 4).

The function should either choose to NOT consider anything about inlineAsm() or should provide flexibility to consider. In latter case, it is not useful and we have to check !callee everywhere.

Typo: In latter case, it is useful than to check !callee everywhere.

[rampitec]

llvm/lib/IR/Instructions.cpp

289

Right now it considers anything which is non inline asm got to this point as an indirect call. You are breaking it. Option wrt inline asm shall not affect non inline asm.

[madhur13490]

llvm/lib/IR/Instructions.cpp

289

InlineAsmMayHaveIndirectCall is "false" by default so no breakage and the function continue to work as it is today for all users (condition1 and 2). If you pass "true" then you get new behavior which this patch proposes.

[madhur13490]

@nickdesaulniers Hi Nick, referring D77600, I feel clients should have the ability to consider treatment of inlineAsm(). This patch adds an optional parameter whose default value is false.

[arsenm]

In D98780#2632576, @madhur13490 wrote:

In D98780#2632539, @arsenm wrote:

I don't think this should be done. The point of the function is to check if the IR instruction is an indirect call. inline is a different entity

But in that case the current function should not even do anything related to inlineAsm(). Treating the presence of inlineAsm is different for difference clients and that is what the flag offers.

The function should return false for inline asm. Inline asm is not an indirect call, and indirect call here refers to the IR construct for an indirect call. The fact that inline assembly may contain code that performs what will be encoded as an indirect call is not the point here. The IR semantic is not concerned with what happens to come out of codegen

[nickdesaulniers]

Perhaps a test case could demonstrate better what you're trying to do?

llvm/include/llvm/IR/InstrTypes.h

public:
 
   /// Returns the function called, or null if this is an
   /// indirect function invocation.
   Function *getCalledFunction() const {
     return dyn_cast_or_null<Function>(getCalledOperand());
   }
 
   /// Return true if the callsite is an indirect call.
-  bool isIndirectCall() const;
+  /// Optionally consider if inline asm may have an indirect call.
+  bool isIndirectCall(const bool InlineAsmMayHaveIndirectCall = false) const;
 
   /// Determine whether the passed iterator points to the callee operand's Use.
   bool isCallee(Value::const_user_iterator UI) const {
     return isCallee(&UI.getUse());
   }
 
   /// Determine whether this Use is the callee operand's Use.
   bool isCallee(const Use *U) const { return &getCalledOperandUse() == U; }

llvm/lib/IR/Instructions.cpp

 
 Function *CallBase::getCaller() { return getParent()->getParent(); }
 
 unsigned CallBase::getNumSubclassExtraOperandsDynamic() const {
   assert(getOpcode() == Instruction::CallBr && "Unexpected opcode!");
   return cast<CallBrInst>(this)->getNumIndirectDests() + 1;
 }
 
-bool CallBase::isIndirectCall() const {
+bool CallBase::isIndirectCall(const bool InlineAsmMayHaveIndirectCall) const {
   const Value *V = getCalledOperand();
   if (isa<Function>(V) || isa<Constant>(V))
     return false;
-  return !isInlineAsm();
+  return InlineAsmMayHaveIndirectCall == isInlineAsm();
 }
 
 /// Tests if this call site must be tail call optimized. Only a CallInst can
 /// be tail call optimized.
 bool CallBase::isMustTailCall() const {
   if (auto *CI = dyn_cast<CallInst>(this))
     return CI->isMustTailCall();
   return false;