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

Mon Mar 23 02:10:42 PDT 2020

samparker marked an inline comment as done.
samparker added inline comments.

================
Comment at: llvm/lib/Target/ARM/ARMLowOverheadLoops.cpp:538
+
+static bool retainsPreviousValue(const MachineInstr &MI) {
+  switch (MI.getOpcode()) {
----------------
SjoerdMeijer wrote:
> samparker wrote:
> > 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'.
> Thanks for pointing this out.
> 
> Just a nit/suggestion for the function name then: 
> `retainsPreviousValue` -> `laneRetainsPreviousValue`
> 
> A few questions about the list below. Is this list meant to be complete? >From looking at the ArmARM a bit more for this behaviour, it looks like there are more, the first search result e.g.  showed VQMOVN. Are we for example not interested in this one? Also curious if it would be worth describing this property in a different way, or if this switch will do.
> 
It is supposed to be complete, so thanks! Which also reminds me that there's no tests for any movn either!

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

https://reviews.llvm.org/D76235