[PATCH] D31801: Performance enhancements for Cavium ThunderX2 T99

[Stefan Teleman via Phabricator via llvm-commits]

steleman added inline comments.


================
Comment at: lib/Target/AArch64/AArch64ISelLowering.cpp:7624
+  case AArch64Subtarget::ThunderX2T99:
+  case AArch64Subtarget::ThunderXT88:
+    if (VT.isFloatingPoint() && VT.isVector())
----------------
kristof.beyls wrote:
> steleman wrote:
> > rengolin wrote:
> > > This smells like a target feature. :)
> > I can definitely re-write it as a TargetFeature.
> > 
> my 2 cents, hoping someone with more experience in this area can help further:
> 
> IIUC, the effect of this change is that also for vector types, this function now returns true, when targeting ThunderX2T99 or ThunderXT88?
> 
> I can see that for different cores/micro-architectures a different response could be wanted here, also depending on the types involved.
> Since the answer could be different dependent on the type, I'm guessing that this won't map nicely to a target feature.
> Maybe this information needs to become a subtarget hook somehow?
> 
> Also, please upload the next iteration of this patch with lots of context (e.g. git diff -U9999) - that makes reviewing it a bit easier.
> 
> 
> IIUC, the effect of this change is that also for vector types, this function now returns true, when targeting ThunderX2T99 or ThunderXT88?

Correct, for floating-point vector types.

> Since the answer could be different dependent on the type, I'm guessing that this won't map nicely to a target feature.

> Since the answer could be different dependent on the type, I'm guessing that this won't map nicely to a target feature.
> Maybe this information needs to become a subtarget hook somehow?

I think it depends on
- how many other micro-architectures (besides Cavium T88/T99) want/need this sub-feature
- how many different types would need different behaviors

So, in my mind, this comes down to where exactly does this complexity reside: in this function, or in a separate hook someplace else.

I don't have a clear answer to this question because I don't know what other micro-architectures need or want with respect to FMA.











Repository:
  rL LLVM

https://reviews.llvm.org/D31801