john.brawn planned changes to this revision. john.brawn added inline comments.
================ Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:1484 + // of the vector comparison instructions. + setOperationAction(ISD::STRICT_FSETCCS, VT, Expand); + // FIXME: We could potentially make use of the vector comparison instructions ---------------- dmgreen wrote: > john.brawn wrote: > > dmgreen wrote: > > > Can you split this into a separate patch? I know I sound like a broken > > > record, but it doesn't seem to be related to the converts below. > > > > > > Also pre-committing as much of the test that works as possible would cut > > > it down from this patch quite a bit. > > Instead of a separate patch just for these two, it would probably make more > > sense to move them into D114946 with the rest of the setOperationAction > > lines. > > > > On the test, without the changes in this patch it hits an assertion failure > > so as a separate commit before this it wouldn't be able to test anything. > D114946 is already pretty big :) > > What about the add_v4f32 test (for example) requires the changes in this > patch? That's what I meant by "precommit as much as possible". > > My other question about this was going to be - why can't we use the vector > instructions for STRICT_FSETCCS? The FCMGE and FCMGT look like they would set > exception flags to me, but I may be misunderstanding some part of it. > D114946 is already pretty big :) A bit, though only in terms of number of lines. Conceptually it fairly simple, as it's almost all "strict_xyz can be handled the same as (non-strict) xyz". > What about the add_v4f32 test (for example) requires the changes in this > patch? That's what I meant by "precommit as much as possible". I _could_ try and disentangle the parts that compile without this patch, but it seems like more work than it's worth. > My other question about this was going to be - why can't we use the vector > instructions for STRICT_FSETCCS? The FCMGE and FCMGT look like they would set > exception flags to me, but I may be misunderstanding some part of it. Hmm, it looks like the FCMXY instructions are inconsistent in their handling of NaNs. FCMEQ performs a quiet comparison (only signalling NaNs raise an exception) like FCMP and STRICT_FSETCC, FCMGT etc. performs a signalling comparison (all NaNs raise an exception) like FCMPE and STRICT_FSETCCS. I'll adjust the comments (and also move this to D114946). CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117795/new/ https://reviews.llvm.org/D117795 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
