This is an archive of the discontinued LLVM Phabricator instance.

AMDGPU/GlobalISel: Add floating point med3 combine
ClosedPublic

Authored by Petar.Avramovic on Oct 23 2020, 8:42 AM.

Details

Summary

Add floating point version of med3 combine.
Source is fminnum(fmaxnum(Val, K0), K1) or fmaxnum(fminnum(Val, K1), K0)
where K0 and K1 are constants and K0 <= K1.

Diff Detail

Event Timeline

Herald added a project: Restricted Project. · View Herald TranscriptOct 23 2020, 8:42 AM
Petar.Avramovic requested review of this revision.Oct 23 2020, 8:42 AM

Missing tests for f16 and v2f16 cases

llvm/lib/Target/AMDGPU/AMDGPUPostLegalizerCombiner.cpp
174–175

Might as well handle this now

181–182

You can directly pass the APFloat to is the overload of isInlineConstant

llvm/lib/Target/AMDGPU/AMDGPUPostLegalizerCombiner.cpp
174–175

I am not really sure I know correct way to do it. Also this combine heavily depends on legalizer and primarily completing remaining of SNaN checks in isKnownNeverSNaN.

foad added inline comments.Oct 27 2020, 3:44 AM
llvm/lib/Target/AMDGPU/AMDGPUPostLegalizerCombiner.cpp
172

This will probably warn that Info is unused in a Release build.

Petar.Avramovic retitled this revision from AMDGPU/GlobalISel: Add floating point med3 combine for IEEE=false to AMDGPU/GlobalISel: Add floating point med3 combine.
Petar.Avramovic edited the summary of this revision. (Show Details)

Rebase/rework.

arsenm added inline comments.Jun 30 2021, 6:03 PM
llvm/lib/Target/AMDGPU/AMDGPURegBankCombiner.cpp
179 ↗(On Diff #352504)

Like in the other patches, why do you need to re-query getConstantFPVRegVal? Why doesn't the constant matcher just directly return the constant?

Use matcher that returns both VReg and APFloat from G_FCONSTANT.

Rebase and ping.

arsenm added inline comments.Dec 1 2021, 3:55 PM
llvm/lib/Target/AMDGPU/AMDGPURegBankCombiner.cpp
192 ↗(On Diff #391135)

I don't see why to make these optional if they won't be set or used if the matcher return false

209 ↗(On Diff #391135)

This should be cached in the helper?

Cache SIInstrInfo in the helper.

Petar.Avramovic added inline comments.Dec 2 2021, 8:52 AM
llvm/lib/Target/AMDGPU/AMDGPURegBankCombiner.cpp
192 ↗(On Diff #391135)

Util helpers and matchers use Optional.

arsenm accepted this revision.Dec 2 2021, 9:10 AM
This revision is now accepted and ready to land.Dec 2 2021, 9:10 AM
This revision was landed with ongoing or failed builds.Dec 3 2021, 4:02 AM
This revision was automatically updated to reflect the committed changes.