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

[Stanislav Mekhanoshin via Phabricator via llvm-commits]

rampitec requested changes to this revision.
rampitec added inline comments.


================
Comment at: llvm/lib/IR/Instructions.cpp:289
     return false;
-  return !isInlineAsm();
+  return InlineAsmMayHaveIndirectCall == isInlineAsm();
 }
----------------
madhur13490 wrote:
> madhur13490 wrote:
> > rampitec wrote:
> > > madhur13490 wrote:
> > > > rampitec wrote:
> > > > > It will give wrong answer for non-asm. "return !isInlineAsm() || !InlineAsmMayHaveIndirectCall;"
> > > > 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.
> > > > 
> > > 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;".
> > 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. 
> > 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.
> 
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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D98780/new/

https://reviews.llvm.org/D98780