[PATCH] D107823: [DebugInfo][InstrRef] Honour too-much-debug-info cut-outs in InstrRefBasedLDV

[Djordje Todorovic via Phabricator via llvm-commits]

djtodoro added inline comments.


================
Comment at: llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp:3658
+  // however we should cut our losses.
+  if (MaxNumBlocks > (int)InputBBLimit && VarAssignCount > InputDbgValLimit) {
+    LLVM_DEBUG(dbgs() << "Disabling InstrRefBasedLDV: " << MF.getName()
----------------
jmorse wrote:
> djtodoro wrote:
> > jmorse wrote:
> > > djtodoro wrote:
> > > > nit: I guess the `(int)` casting isn't necessary here.
> > > Alas, there's a PPC buildbot out there running with -Werror that fails to build without this :(.
> > > 
> > > It really is an int-to-unsigned comparison, as I'm using '-1' as an error value of MaxNumBlocks earlier in the function, after which it's always positive. It's less than ideal, but shouldn't present a problem.
> > > 
> > > ... although thinking about it now, it would be better to cast the signedMaxNumBlocks to unsigned, as InputBBLimit is user controlled, and they'd be surprised if a very large max-blocks-limit inverted its behaviour. I'll fold that in when landing again.
> > In the `produceMLocTransferFunction()` it becomes `unsigned`, so that is why I thought those are the same type. This is OK. :)
> > 
> > >... although thinking about it now, it would be better to cast the signedMaxNumBlocks to unsigned, as InputBBLimit is user controlled, and they'd be surprised if a very large max-blocks-limit inverted its behaviour. I'll fold that in when landing again.
> > 
> > Cool, should we do a similar thing on the VarLocBasedLDV side?
> > Cool, should we do a similar thing on the VarLocBasedLDV side?
> 
> It's all unsigned over there, shouldn't be any issues.
cool, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107823