[PATCH] D102942: Remove or use variables which are unused but set.

David Blaikie via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri May 21 14:56:36 PDT 2021


dblaikie accepted this revision.
dblaikie added a comment.
This revision is now accepted and ready to land.

Sure, all sounds good - if you can, please reach out to the authors of any of the semantics changing changes (the ones related to the `Changed` values in transformations) to see if they could add missing test coverage.



================
Comment at: llvm/lib/Target/AArch64/GISel/AArch64PostSelectOptimize.cpp:183
     Changed |= optimizeNZCVDefs(BB);
-  return true;
+  return Changed;
 }
----------------
Might want to CC/respond to whoever wrote this to see if someone missed test coverage for this code.


================
Comment at: llvm/lib/Target/ARM/ARMLowOverheadLoops.cpp:1247-1250
   if (LoLoop.Preheader)
     LoLoop.Start = SearchForStart(LoLoop.Preheader);
   else
+    return Changed;
----------------
Maybe in a separate patch would be good to switch this around to:
```
if (!LoLoop.Preheader)
  return Changed;
LoLoop.Start = SearchForStart(LoLoop.Preheader);
```


================
Comment at: llvm/lib/Target/ARM/ARMLowOverheadLoops.cpp:1250
   else
-    return false;
+    return Changed;
 
----------------
Also might be worth reaching out to authors to check that this change is intended & possibly tested.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102942/new/

https://reviews.llvm.org/D102942



More information about the llvm-commits mailing list