[PATCH] D127134: [llvm] Add DW_CC_nocall to function debug metadata when either return values or arguments are removed

[Venkata Ramanaiah Nalamothu via Phabricator via llvm-commits]

RamNalamothu marked 2 inline comments as done.
RamNalamothu added inline comments.


================
Comment at: llvm/lib/Transforms/IPO/DeadArgumentElimination.cpp:1085-1088
+    for (uint8_t I = 0; I < SP->getNumOperands(); ++I)
+      if (SP->getOperand(I).get() == SPTy)
+        SP->replaceOperandWith(I,
+                               MDNode::replaceWithPermanent(std::move(Temp)));
----------------
scott.linder wrote:
> RamNalamothu wrote:
> > RamNalamothu wrote:
> > > dblaikie wrote:
> > > > Writing a loop to do this seems unfortunate when it should always be a known operand number - perhaps there's some nicer way to do this? (while keeping it maintainable/not hardcoding an operand number in this code)
> > > > 
> > > > Are there other examples of field updates we could draw inspiration from?
> > > I couldn't figure out any other simpler way to do this.
> > > 
> > > I don't think the operand numbers are fixed/known (e.g. see llvm/test/DebugInfo/Generic/array.ll and llvm/test/DebugInfo/Generic/enum.ll).
> > > 
> > > And in all the code browsing I did, I haven't seen any other examples that could help with this. Please correct me if I missed something.
> > > I don't think the operand numbers are fixed/known (e.g. see llvm/test/DebugInfo/Generic/array.ll and llvm/test/DebugInfo/Generic/enum.ll).
> > 
> > I mean, looking at `DISubprogram` node in those LIT tests convey that the `type` operand can be anywhere and not at a fixed position.
> > 
> > 
> Can you define a new method `DISubprogram::replaceType(DIType *T) { replaceOperandWith(4, T); }` and use that? There is already e.g. `void DISubprogram::replaceUnit(DICompileUnit *CU) { replaceOperandWith(5, CU); }`
> 
> With either implementation, are we only ever mutating non-uniqued `DISubprogram` nodes? I believe `DISubprogram` nodes for declarations may be uniqued, but it seems like this pass can only be making changes when the definition is available, so we are safe?
Thanks @scott.linder for the suggestion. Not sure how I missed this.

Yes, the DAE works only on the definitions and I have added an `assert(isDistinct())` to the new method as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127134