[PATCH] D114946: [AArch64] Add instruction selection for strict FP

[John Brawn via Phabricator via llvm-commits]

john.brawn added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:1483
+
+  // FIXME: We could potentially make use of the vector comparison instructions
+  // for STRICT_FSETCC and STRICT_FSETCSS, but there's a number of
----------------
dmgreen wrote:
> john.brawn wrote:
> > dmgreen wrote:
> > > FCmp are a pretty common operation. Is it not something we would want to support, without expanding?
> > > 
> > > Don't we have intrinsics that would expect to become a single vector operation?
> > Yes, it's something we would want to support, hence the FIXME. And with regards to intrinsics, generating code that's correct is more important than using a single instruction.
> In my experience, FIXME's are things people leave in the code because they have no plan of fixing them. Correct code is of course important, but people also have an expectation of a certain level of performance from the intrinsics.
Yes, I'm not planning on fixing this. My primary goal here is to have code generation that is valid, and secondarily when we have some existing ISel patterns that apply to non-strict and would also be valid for strict adjust them to work with strict. Vector STRICT_FSETCC/STRICT_FSETCCS are more complex than this, hence leaving them as Expand (which is the default for strict vector operations anyway).


================
Comment at: llvm/lib/Target/AArch64/AArch64InstrInfo.td:4230
           (FCVTLv8i16 V128:$Rn)>;
-def : Pat<(v2f64 (fpextend (v2f32 V64:$Rn))), (FCVTLv2i32 V64:$Rn)>;
+def : Pat<(v2f64 (any_fpextend (v2f32 V64:$Rn))), (FCVTLv2i32 V64:$Rn)>;
 
----------------
dmgreen wrote:
> None of the test below seem to be Neon. Are they tested somewhere else?
D117795 adds neon tests.


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

https://reviews.llvm.org/D114946