[PATCH] D79872: [MachineVerifier] Use the for Range loop to instead llvm::any_of

Zhang Kang via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed May 13 09:10:55 PDT 2020


ZhangKang created this revision.
ZhangKang added reviewers: echristo, MaskRay, jyknight, PowerPC, hfinkel, nemanjai, jsji, steven.zhang.
ZhangKang added a project: LLVM.
Herald added subscribers: wuzish, hiraditya.
ZhangKang edited the summary of this revision.
ZhangKang added a reviewer: nickdesaulniers.
ZhangKang edited the summary of this revision.
MaskRay added inline comments.


================
Comment at: llvm/lib/CodeGen/MachineVerifier.cpp:172
+        bool Changed = false;
+        for (const auto &I: RS)
+          Changed |= addRequired(I);
----------------
s/const auto &/unsigned/


In the patch https://reviews.llvm.org/D78849, it uses `llvm::any_of ` to instead of for loop to simplify the function `addRequired ()`:
Below function

  bool addRequired(const RegSet &RS) {
     bool changed = false;
     for (RegSet::const_iterator I = RS.begin(), E = RS.end(); I != E; ++I)
       if (addRequired(*I))
         changed = true;
     return changed;
  }

will be simplified to:

  bool addRequired(const RegSet &RS) {
       return llvm::any_of(
           RS, [this](unsigned Reg) { return this->addRequired(Reg); });
  }

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 all element to call `addRequired(Reg)`.

This bug has caused 248 cases error for the Machine Verfication of LiveVariable pass.
This patch uses for range loop to fix llvm::any_of bug.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D79872

Files:
  llvm/lib/CodeGen/MachineVerifier.cpp


Index: llvm/lib/CodeGen/MachineVerifier.cpp
===================================================================
--- llvm/lib/CodeGen/MachineVerifier.cpp
+++ llvm/lib/CodeGen/MachineVerifier.cpp
@@ -168,14 +168,18 @@
 
       // 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 (const auto &I: RS)
+          Changed |= addRequired(I);
+        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;
       }
 
       // Live-out registers are either in regsLiveOut or vregsPassed.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D79872.263736.patch
Type: text/x-patch
Size: 936 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200513/14c09e85/attachment-0001.bin>


More information about the llvm-commits mailing list