This is an archive of the discontinued LLVM Phabricator instance.

[AMDGPU] Enable ds_min/ds_max on more subtargets
ClosedPublic

Authored by Joe_Nash on Aug 31 2021, 6:58 AM.

Details

Summary

Adds patterns for f64 ds_min/ds_max. Shrinks HasLDSFPAtomics
scope to enable f32.

Diff Detail

Event Timeline

Joe_Nash created this revision.Aug 31 2021, 6:58 AM
Joe_Nash requested review of this revision.Aug 31 2021, 6:58 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 31 2021, 6:58 AM
foad added a comment.Aug 31 2021, 7:35 AM

Shrinks HasLDSFPAtomics scope to enable f32.

"... to enable f32 add"? Maybe update the description of the feature to reflect this?

foad added inline comments.Aug 31 2021, 8:20 AM
llvm/test/CodeGen/AMDGPU/lds-atomic-add.ll
1 ↗(On Diff #369691)

This file looks like a subset of the existing test/CodeGen/AMDGPU/lds_atomic_f32.ll?

Joe_Nash updated this revision to Diff 369709.Aug 31 2021, 8:31 AM

update name of feature since it now only controls DS_ADD_F32

Joe_Nash marked an inline comment as done.Aug 31 2021, 8:33 AM

Shrinks HasLDSFPAtomics scope to enable f32.

"... to enable f32 add"? Maybe update the description of the feature to reflect this?

Agreed

llvm/test/CodeGen/AMDGPU/lds-atomic-add.ll
1 ↗(On Diff #369691)

I split the test because the lds_atomic_fadd is only on gfx8 and gfx9, but the min/max are available on all architectures

foad added inline comments.Aug 31 2021, 8:43 AM
llvm/test/CodeGen/AMDGPU/lds-atomic-add.ll
1 ↗(On Diff #369691)

Oh, I hadn't even noticed that your patch deletes lds_atomic_f32.ll. OK then.

The new files should probably be called lds-atomic-fadd.ll and lds-atomic-fmin-fmax.ll.

Joe_Nash updated this revision to Diff 369716.Aug 31 2021, 8:54 AM
Joe_Nash marked an inline comment as done.

rename tests

This revision is now accepted and ready to land.Aug 31 2021, 9:54 AM
This revision was automatically updated to reflect the committed changes.
llvm/test/CodeGen/AMDGPU/lds-atomic-fmin-fmax.ll