[PATCH] Debug info: Cleanup collectChangingRegs

David Blaikie dblaikie at gmail.com
Wed Aug 6 10:50:02 PDT 2014


On Wed, Aug 6, 2014 at 10:42 AM, Frédéric Riss <friss at apple.com> wrote:
>
> On 06 Aug 2014, at 18:57, David Blaikie <dblaikie at gmail.com> wrote:
>
>> The isDebugValue checking seems like a somewhat arbitrary case of
>> pre-filtering.
>
> It is indeed pre-filtering. I introduced it to match the corresponding test in calculateDbgValueHistory where the ChangingRegs array is checked only for !DbgValue instructions.
>
>> Does it have an observable performance impact?
>
> It was in the noise, otherwise I would have integrated it in the performance patch.
>
>> 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).

- David




More information about the llvm-commits mailing list