[PATCH] D76235: [ARM][LowOverheadLoops] Add checks for narrowing

[Sjoerd Meijer via Phabricator via llvm-commits]

SjoerdMeijer added inline comments.


================
Comment at: llvm/lib/Target/ARM/ARMLowOverheadLoops.cpp:521
 
+static bool canGenerateNonZeros(const MachineInstr &MI) {
+  switch (MI.getOpcode()) {
----------------
samparker wrote:
> SjoerdMeijer wrote:
> > This is becoming a very tricky pass (if it wasn't already that), for a bunch of different reason: the analysis, the transformations, etc. To keep things readable, I am now going to nitpick on comments. I believe there is one school of thought that code should be self-documenting, and comments will always be out of date or misleading, which I think I can mostly agree with. But here in this case, this function, that checks certain opcodes and returning true/false to a function named `canGenerateNonZeros` doesn't tell me much. So my request is here is if you can define what exactly canGenerateNonZeros mean, and why these opcodes.
> >  
> Fair! Here, any MVE instruction that could generate a non-zero result given nothing but zero'd registers should be listed. This allows us to track zeros in means zeros out.
Thanks, makes sense. If you can later add this as comments, that would be much appreciated. I am now going to do a second pass over this patch, let things sink in, and continue the review.


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

https://reviews.llvm.org/D76235