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
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
