[PATCH] D111460: [X86][LoopVectorize] "Fix" `X86TTIImpl::getAddressComputationCost()`

Tue Oct 19 09:40:01 PDT 2021

RKSimon added a comment.

TBH I'd much prefer we focus on fixing X86 interleaved costs, writing a script and comparing different sse levels vs cpus like I did on D103695 <https://reviews.llvm.org/D103695>, even if the llvm-mca numbers aren't great they will give us a better magnitude.

Or maybe improve the fallback costs - for instance by generating costs for the BaseT getInterleavedMemoryOpCost for shuffle and scalarization patterns and returning the lowest.

================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:7061
   Type *PtrTy = ToVectorTy(Ptr->getType(), VF);
+  // FIXME: PtrTy should not be a vector, it's a hack.

----------------
dmgreen wrote:
> pengfei wrote:
> > lebedev.ri wrote:
> > > pengfei wrote:
> > > > It would be more productive if it includes the reason why we can’t change it right away.
> > > > Lack of such explanation led to the original version of the patch that repeats D93129.
> > > I'm open to suggestions as to how this should be amended.
> > Sorry, I don't have a concrete suggestion. I have concerns because I saw disagreement from Dave and didn't see consensus have been made.
> My comments were mostly for ARM/AArch64, which this patch no longer affects. I don't have as much knowledge on X86 matters and no evidence suggesting one way or another whether this is a good idea I'm afraid.
> 
> I don't think this comment is particularly useful though. Personally I would personally just remove this comment and leave the patch to the X86 backend.
@fhahn What happened to D93129 that actually dealt with this?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111460