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

[Sam Parker via Phabricator via llvm-commits]

samparker marked 2 inline comments as done.
samparker added inline comments.


================
Comment at: llvm/lib/Target/ARM/ARMLowOverheadLoops.cpp:521
 
+static bool canGenerateNonZeros(const MachineInstr &MI) {
+  switch (MI.getOpcode()) {
----------------
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.


================
Comment at: llvm/lib/Target/ARM/ARMLowOverheadLoops.cpp:538
+
+static bool retainsPreviousValue(const MachineInstr &MI) {
+  switch (MI.getOpcode()) {
----------------
SjoerdMeijer wrote:
> Same request here: what is `retainsPreviousValue`, and why these opcodes?
All these instructions operate upon half a lane of the source register, writing to the destination, but they also leave the other half of the destination register untouched. The reference manual uses this sentence: 'The other half of the destination vector element retains its previous value'.


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

https://reviews.llvm.org/D76235