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

[Scott Linder via Phabricator via llvm-commits]

scott.linder 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)));
----------------
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?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127134