[PATCH] D30835: [DebugInfo][X86] Teach Optimize LEAs pass to handle debug values

[Daniel Berlin via llvm-commits]

No.
This code as written is safe against the *current* use changing, but the
auto version is not.
The range loop is essentially:
*{*
*auto && __range =* range_expression *;*
*for (auto __begin =* *begin_expr**,* *__end =* *end_expr**;*
*__begin != __end; ++__begin) {*
range_declaration *= *__begin;*
loop_statement
*} *

*} *

If loop statement destroys begin, you can't move to the next thing properly
(at least, for uses).

This would work if it was:
*{*
*auto && __range =* range_expression *;*
*for (auto __begin =* *begin_expr**,* *__end =* *end_expr**;*
*__begin != __end;) {*
range_declaration *= *__begin++;*
loop_statement
*} *

*} *


On Fri, Mar 17, 2017 at 10:01 AM, David Blaikie via llvm-commits <
llvm-commits at lists.llvm.org> wrote:

>
>
> On Fri, Mar 17, 2017 at 9:57 AM Andrew Ng via Phabricator <
> reviews at reviews.llvm.org> wrote:
>
>> andrewng added a comment.
>>
>> Thanks for the review.
>>
>>
>>
>> ================
>> Comment at: lib/Target/X86/X86OptimizeLEAs.cpp:567-568
>> +        unsigned LastVReg = Last.getOperand(0).getReg();
>> +        for (auto UI = MRI->use_nodbg_begin(LastVReg),
>> +                  UE = MRI->use_nodbg_end();
>>               UI != UE;) {
>> ----------------
>> dblaikie wrote:
>> > I think you can use:
>> >
>> >   for (MachineOperand &MO : MRI->use_nodbg_operands(LastVReg))
>> >
>> > here, probably?
>> Yes, at first I thought so too, but inside the loop there is:
>> ```
>> MO.setReg(First.getOperand(0).getReg());
>> ```
>> which could affect the use iterator. So the original code which
>> increments the use iterator before any changes is safer.
>>
>
> Not quite sure how one could be safer than the other - they're the same
> code, aren't they? (range-for and the manually written for loop look like
> they have identical semantics)
>
>
>>
>>
>> https://reviews.llvm.org/D30835
>>
>>
>>
>>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170317/1427cdef/attachment.html>