This is an archive of the discontinued LLVM Phabricator instance.

[AArch64][SME]: Generate streaming-compatible code for fp compares.
ClosedPublic

Authored by hassnaa-arm on Nov 24 2022, 7:00 AM.

Details

Summary

To generate code compatible to streaming mode:

    • enable expanding ISD::SETUEQ to avoid custom-lowering setcc to setcc_merge_zero which cause a crash while instruction selection because there is no pattern match for it.
  • Testing files:
    • fp-compares.ll

Diff Detail

Event Timeline

hassnaa-arm created this revision.Nov 24 2022, 7:00 AM
Herald added a project: Restricted Project. · View Herald TranscriptNov 24 2022, 7:00 AM
hassnaa-arm requested review of this revision.Nov 24 2022, 7:00 AM
Herald added a project: Restricted Project. · View Herald TranscriptNov 24 2022, 7:00 AM
sdesmalen added inline comments.Nov 24 2022, 7:10 AM
llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
1664

SETLT and SETLE can be removed from the list (I mentioned that on https://reviews.llvm.org/D136858#inline-1332721 as well)

llvm/test/CodeGen/AArch64/sve-streaming-mode-fixed-length-fp-compares.ll
10

Why does this file not have CHECK lines for each test?

hassnaa-arm marked an inline comment as done.

Remove SETLT and SETLE

llvm/test/CodeGen/AArch64/sve-streaming-mode-fixed-length-fp-compares.ll
10

because this is a new test, and the test file was crashing, so I couldn't generate new code for the precursory patch.

hassnaa-arm edited the summary of this revision. (Show Details)Nov 24 2022, 8:15 AM
david-arm added inline comments.Nov 25 2022, 5:46 AM
llvm/test/CodeGen/AArch64/sve-streaming-mode-fixed-length-fp-compares.ll
10

In this case @hassnaa-arm I think it probably makes sense to add this test only in this patch, otherwise the pre-commit patch will cause build failures. I know that if you commit them in sequence quickly it shouldn't matter much, but if for anyone reason you have to revert this patch then you'll also have to revert the pre-commit too.

39

I'm a bit confused by the old CHECK lines - it looks like this already used SVE perfectly fine before? Also, it looks like this is assuming a minimum SVE vector length of 256 bits because there is only a single fcmeq instruction for a <16 x half> operation.

I noticed similar things in the other tests too - would you mind taking a look into this and finding out what's happening here?

hassnaa-arm added inline comments.Nov 25 2022, 5:53 AM
llvm/test/CodeGen/AArch64/sve-streaming-mode-fixed-length-fp-compares.ll
10

okay, that makes sense.thanks.

39

It used SVE perfectly because it was using -aarch64-sve-vector-bits-min=256.
I couldn't remove that flag and rerun the tests because I crashes when I do that.
So, the perfect solution as you said is to add this file in the new patch only not the precursory patch.

Remove fp-compares.ll file from precursory patch.

This revision is now accepted and ready to land.Nov 25 2022, 7:43 AM
Matt added a subscriber: Matt.Nov 27 2022, 10:35 AM
hassnaa-arm retitled this revision from [AArch64][SME]: Generate streaming-compatible code for int/fp compares. to [AArch64][SME]: Generate streaming-compatible code for fp compares..Nov 28 2022, 1:56 AM
sdesmalen accepted this revision.Nov 28 2022, 1:57 AM
This revision was landed with ongoing or failed builds.Nov 28 2022, 3:22 AM
This revision was automatically updated to reflect the committed changes.