[PATCH] D24857: [ARM] Assign cost of scaling used in addressing mode for ARM cores

[James Molloy via llvm-commits]

jmolloy accepted this revision.
jmolloy added a reviewer: jmolloy.
jmolloy added inline comments.


================
Comment at: lib/Target/ARM/ARMISelLowering.cpp:11604
+  if (isLegalAddressingMode(DL, AM, Ty, AS)) {
+    if (Subtarget->hasFPAO())
+      return AM.Scale < 0 ? 1 : 0; // positive offsets execute faster
----------------
rengolin wrote:
> jmolloy wrote:
> > rengolin wrote:
> > > This is not really a cost model... but I don't have a better idea. This is a very specific feature.
> > > 
> > > @jmolloy, are you happy with this?
> > I know you asked for it to be a feature flag, but... I'm not particularly keen on it as a feature flag. Are we moving towards feature flags for cost model decisions now? :/
> > 
> > Wouldn't something like:
> > 
> >    switch (Subtarget->getCPUName()) {
> >    default: return 0;
> >    case Cortex-R52:
> >       return AM.Scale < 0 ? 1 : 0;
> >    }
> > 
> > be alright? It's more cost-model-like...
> Good lord, no! We're moving away from core checks. :)
Huh. Well it LGTM then. The cost model (1 or 0) doesn't seem particularly bad.


https://reviews.llvm.org/D24857