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

David Blaikie via llvm-commits llvm-commits at lists.llvm.org
Fri Mar 17 13:17:52 PDT 2017


Ah, thanks for the pointer - I was looking for the usual "invalidates
'end'" problem and the fact that the code was saving end and not
recomputing it lead me to think that wasn't an issue, then. I guess this
range has a blessed/universal/constant end that doesn't need to be
recomputed? Ah, yep, I see it (defusechain_iterator) does, it goes to null.

Thanks for explaining/catching that!

On Fri, Mar 17, 2017 at 1:09 PM Daniel Berlin <dberlin at dberlin.org> wrote:

> 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/883f1e28/attachment.html>


More information about the llvm-commits mailing list