[llvm] aedb661 - [MachineVerifier] Use the for_range loop to instead llvm::any_of

[David Blaikie via llvm-commits]

On Thu, May 14, 2020 at 7:36 PM Kang Zhang via llvm-commits <
llvm-commits at lists.llvm.org> wrote:

>
> Author: Kang Zhang
> Date: 2020-05-15T02:35:33Z
> New Revision: aedb6615a8d08d7dfeba17bf22ff4c6fd104cadc
>
> URL:
> https://github.com/llvm/llvm-project/commit/aedb6615a8d08d7dfeba17bf22ff4c6fd104cadc
> DIFF:
> https://github.com/llvm/llvm-project/commit/aedb6615a8d08d7dfeba17bf22ff4c6fd104cadc.diff
>
> LOG: [MachineVerifier] Use the for_range loop to instead llvm::any_of
>
> Summary:
> In the patch D78849, it uses llvm::any_of to instead of for loop to
> simplify the function addRequired().
> It's obvious that above code is not a NFC conversion. Because any_of
> will return if any addRequired(Reg) is true immediately, but we want
> every element to call addRequired(Reg).
>
> This patch uses for_range loop to fix above any_of bug.
>
> Reviewed By: MaskRay, nickdesaulniers
>
> Differential Revision: https://reviews.llvm.org/D79872
>
> Added:
>
>
> Modified:
>     llvm/lib/CodeGen/MachineVerifier.cpp
>
> Removed:
>
>
>
>
> ################################################################################
> diff  --git a/llvm/lib/CodeGen/MachineVerifier.cpp
> b/llvm/lib/CodeGen/MachineVerifier.cpp
> index 5733798fdc9c..f626c1291607 100644
> --- a/llvm/lib/CodeGen/MachineVerifier.cpp
> +++ b/llvm/lib/CodeGen/MachineVerifier.cpp
> @@ -168,14 +168,18 @@ namespace {
>
>        // Same for a full set.
>        bool addRequired(const RegSet &RS) {
> -        return llvm::any_of(
> -            RS, [this](unsigned Reg) { return this->addRequired(Reg); });
> +        bool Changed = false;
> +        for (unsigned Reg : RS)
> +          Changed |= addRequired(Reg);
> +        return Changed;
>        }
>
>        // Same for a full map.
>        bool addRequired(const RegMap &RM) {
> -        return llvm::any_of(
> -            RM, [this](const auto &P) { return
> this->addRequired(P.first); });
> +        bool Changed = false;
> +        for (const auto &I : RM)
> +          Changed |= addRequired(I.first);
> +        return Changed;
>

FWIW, I'd generally encouraged capturing by "[&]" for any
locally-scoped/non-escaping lambdas (especially those used wholely within
the same statement they're defined in) - like normal scopes (for/if/etc)
it's quite reasonable/readable for a lambda scope to have access to any/all
enclosing variables without having to explicitly enumerate them.


>        }
>
>        // Live-out registers are either in regsLiveOut or vregsPassed.
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> https://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/20200518/5fd5a6fc/attachment.html>