[PATCH] D71109: [ARM] Disable VLD4 under MVE

Dave Green via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Dec 6 08:01:38 PST 2019


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


================
Comment at: llvm/lib/Target/ARM/ARMISelLowering.cpp:147
+MVEMaxSupportedInterleaveFactor("mve-max-interleave-factor", cl::Hidden,
+  cl::desc("Maximum VLDn for MVE. Defaults to 2"),
+  cl::init(2));
----------------
SjoerdMeijer wrote:
> Nit: not sure we need to say "Defaults" to 2 as I don't see much prior art of mentioning the default value (it would also require to keep the message up to date with the value when that gets changed, although that is of course not a really difficult problem).
I thought I had seen this in more places, but you are probably right, I'll take that bit out.


================
Comment at: llvm/test/Transforms/LoopVectorize/ARM/mve-vldn.ll:4
+; RUN: opt -loop-vectorize -mve-max-interleave-factor=2 < %s -S -o - | FileCheck %s --check-prefixes=CHECK,CHECK-2,CHECK-NO4
+; RUN: opt -loop-vectorize -mve-max-interleave-factor=4 < %s -S -o - | FileCheck %s --check-prefixes=CHECK,CHECK-2,CHECK-4
+
----------------
SjoerdMeijer wrote:
> Perhaps silly question, but what happens when we pass `-mve-max-interleave-factor=42`, i.e. a value > 4? Is input checked? Do we need to mention that acceptable values are 1, 2, and 4?
I was going for not caring about the internal compiler options being given strange values. I think it would likely hit an assert if you put it too high. Hopefully something we don't need to worry about in any case?

The nice thing about an option like this is that we can also set it to 1 if we need to disable the vldn's entirely.


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

https://reviews.llvm.org/D71109





More information about the llvm-commits mailing list