This is an archive of the discontinued LLVM Phabricator instance.

AMDGPU: Optimize set_rounding if input is known to fit in 2 bits
Needs ReviewPublic

Authored by arsenm on Jun 19 2023, 3:29 AM.

Details

Reviewers
foad
rampitec
jhuber6
Pierre-vh
b-sumner
Group Reviewers
Restricted Project
Summary

We don't need to figure out the weird extended rounding modes or
handle offsets to keep the lookup table in 64-bits.

Diff Detail

Event Timeline

arsenm created this revision.Jun 19 2023, 3:29 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 19 2023, 3:29 AM
arsenm requested review of this revision.Jun 19 2023, 3:29 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 19 2023, 3:29 AM
Herald added a subscriber: wdng. · View Herald Transcript

Code LGTM but I'm not well-versed in this kind of floating point magic so I will leave it up to someone that knows more to approve :)

llvm/lib/Target/AMDGPU/SIISelLowering.cpp
3639–3641

nit: inline UseReducedTable?

arsenm added inline comments.Jul 11 2023, 1:36 PM
llvm/lib/Target/AMDGPU/SIISelLowering.cpp
3639–3641

Then you lose the name-as-comment?

Did you run adequate testing on this (OCL conformance maybe?) ? If so then I can approve it I think

llvm/lib/Target/AMDGPU/SIISelLowering.cpp
3657–3670

Does that take a lot of effort to add? Why not add it now?

foad added inline comments.Jul 27 2023, 3:57 AM
llvm/lib/Target/AMDGPU/SIISelLowering.cpp
3646

This is just AMDGPU::FltRoundToHWConversionTable & 0xFFFF isn't it? I'm not sure it's worth having all the boilerplate in SIModeRegisterDefaults.cpp to define another table.

Did you run adequate testing on this (OCL conformance maybe?) ? If so then I can approve it I think

There's no wired up user of this

llvm/lib/Target/AMDGPU/SIISelLowering.cpp
3657–3670

There are enough cases to think about in one patch as it is

arsenm updated this revision to Diff 555136.Aug 31 2023, 12:12 PM

Use one table and rebase