[PATCH] D110259: [SVE][Analysis] Tune the cost model according to the tune-cpu attribute

Tue Oct 12 06:30:02 PDT 2021

dmgreen added inline comments.

================
Comment at: llvm/lib/Target/AArch64/AArch64Subtarget.cpp:207
+
+  if (TuneCPU == "neoverse-v1")
+    VScaleForTuning = 2;
----------------
david-arm wrote:
> dmgreen wrote:
> > I don't think these should be needed, as the above ARMProcFamily should be based on TuneCPU after D110258/D111551.
> Hi @dmgreen, I was quite worried that D111551 will hold up this patch and I don't really see why it has to, therefore I have kept them independent. I was thinking that once D111551 lands then I can fix up this code and use ARMProcFamily instead. We'd really like to get the cost model changes in asap because it's preventing vectorisation on machines with SVE hardware.
> 
> I feel that D111551 is a nice thing to have, but not a requirement for the cost model. I also expect it will take a lot longer to get approved. :)
I'm not sure I see why we need to rush with a suboptimal solution. SVE autovec was not enabled, and this patch just enabled some extra gather/scatters and reductions. D111551 doesn't look like it should take too long either, but maybe I'm being optimistic there.

If you want it to land sooner, you could just remove all the references to tune-cpu in this patch. That way it works with -mcpu, the same as every other tuning feature has in llvm for the last 10 year :)

It should then automatically become a tuning features when those other two patches are done.

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

https://reviews.llvm.org/D110259