[PATCH] Debug info: Cleanup collectChangingRegs

[Alexey Samsonov]

On Wed, Aug 6, 2014 at 11:32 AM, Adrian Prantl <aprantl at apple.com> wrote:

>
> On Aug 6, 2014, at 11:11 AM, Frédéric Riss <friss at apple.com> wrote:
>
>
> On 06 Aug 2014, at 19:50, David Blaikie <dblaikie at gmail.com> wrote:
>
> Is there
> some more general test we can do here that would catch more cases of
> MIs that don't have register definitions? (to have a bigger impact and
> make the intent of the code clearer - it's not about skipping debug
> values, it's about skipping non-defining instructions)
>
>
> That’s a question I asked myself. If this test exists, then it could go
> into calculateDbgValueHistory also to get bigger impact.
>
>
> Yeah - then it seems weird to have it out here. Just makes the reader
> ask more questions, I think. Either it matters and it should be in
> calculateDbgValueHistory, as you say, or it doesn't and it shouldn't
> be anywhere because it's just adding cognitive load for readers...
>
> I'd just suggest dropping that (& pestering Alexey about why it was in
> the other place too).
>
>
> Well, in calculateDbgValueHistory the test has to be there as the loop is
> meant to handle DbgValue instructions specifically.
>
>
> It shouldn’t matter in collectChangingRegs(), because a debug value
> doesn’t define any registers. It’s probably marginally faster than going
> through the list of operands, but otherwise safe to drop. The other
> instance in calculateDbgValueHistory() needs to be there for the reason
> mentioned above. I’ll go ahead and commit this (without the first check)
> now.
>

Agree. Thanks for the cleanup!


>
> thanks,
> adrian
>
> Fred
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
>


-- 
Alexey Samsonov
vonosmas at gmail.com
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140806/d0966310/attachment.html>