This is an archive of the discontinued LLVM Phabricator instance.

[AMDGPU] Add f16 to i1 CodeGen patterns.
ClosedPublic

Authored by whchung on Feb 3 2021, 11:58 AM.

Details

Summary

Follow patterns used for f32 and f64 types.

Diff Detail

Event Timeline

whchung created this revision.Feb 3 2021, 11:58 AM
whchung requested review of this revision.Feb 3 2021, 11:58 AM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 3 2021, 11:58 AM

The patterns were found in some MLIR-based applications.

This revision is now accepted and ready to land.Feb 3 2021, 12:01 PM

Should also test globalisel

@arsenm Could you help me understand if my understanding is correct to fulfill your requirement of 'test globalisel'?

  • change the followings under llvm/test/CodeGen/AMDGPU/GlobalISel:
    • inst-select-fptosi.mir
    • legalize-fptosi.mir
    • inst-select-fptoui.mir
    • legalize-fptoui.mir
  • populate FileCheck macros with update_mir_test_checks.py
whchung updated this revision to Diff 321210.Feb 3 2021, 1:28 PM

Add MIR-level tests.

This revision was landed with ongoing or failed builds.Feb 4 2021, 9:44 AM
This revision was automatically updated to reflect the committed changes.
arsenm added inline comments.Feb 4 2021, 9:50 AM
llvm/test/CodeGen/AMDGPU/GlobalISel/inst-select-fptoui.mir
117–122

This isn't testing a target with f16 instructions

whchung added inline comments.Feb 4 2021, 10:19 AM
llvm/test/CodeGen/AMDGPU/GlobalISel/inst-select-fptoui.mir
117–122

@arsenm Could you help review if https://reviews.llvm.org/D96061 addresses your concerns? Thanks.

arsenm added inline comments.Feb 4 2021, 11:01 AM
llvm/test/CodeGen/AMDGPU/GlobalISel/inst-select-fptoui.mir
123–126

Hmm. It looks like these patterns aren't working. The MIR register banks are also not what I would expect. I think this needs some up-front work in the legalizer/regbankselect to handle properly, and would need end to end IR tests

whchung added inline comments.Feb 4 2021, 12:36 PM
llvm/test/CodeGen/AMDGPU/GlobalISel/inst-select-fptoui.mir
123–126

@arsenm I'll take a look into AMDGPULegalizerInfo and AMDGPURegisterBankInfo. I'm not that well-versed in these aspect so I may come up with questions initially.

For end-to-end IR tests, do you feel any additions to fptosi.f16.ll and fptoui.f16.ll in this patch needed?

arsenm added inline comments.Feb 4 2021, 4:18 PM
llvm/test/CodeGen/AMDGPU/GlobalISel/inst-select-fptoui.mir
123–126

Some cases that use the i1 result in a boolean context (e.g. branch or select condition) would be helpful