This is an archive of the discontinued LLVM Phabricator instance.

[AMDGPU] Tweak predicates for image_bvh_intersect_ray instructions
ClosedPublic

Authored by foad on Mar 3 2022, 7:39 AM.

Details

Summary

Don't override SubtargetPredicate since that is already set in the
base classes for the appropriate subtarget like MIMG_gfx10. Use
OtherPredicates instead for consistency with the way we handle
features like HasImageInsts and HasExtendedImageInsts. NFC.

Diff Detail

Event Timeline

foad created this revision.Mar 3 2022, 7:39 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 3 2022, 7:39 AM
foad requested review of this revision.Mar 3 2022, 7:39 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 3 2022, 7:39 AM
Joe_Nash added inline comments.Mar 3 2022, 8:49 AM
llvm/lib/Target/AMDGPU/MIMGInstructions.td
1090

Do you need to concatenate Predicates set in the enclosing scope? Isn't this overriding let OtherPredicates = [HasImageInsts] ?
That may not matter in practice if HasGFX10_AEncoding implies HasImageInsts

foad added inline comments.Mar 3 2022, 9:08 AM
llvm/lib/Target/AMDGPU/MIMGInstructions.td
1090

Yes this relies on HasGFX10_AEncoding being a subset of HasImageInsts. The code just above was already doing this for HasExtendedImageInsts so I thought it was OK like that.

rampitec accepted this revision.Mar 3 2022, 9:44 AM

Regardless if you decide to use both predicates or not this is LGTM.

llvm/lib/Target/AMDGPU/MIMGInstructions.td
1090

This is a list, so you could use both is that is a concern: [HasGFX10_AEncoding, HasImageInsts].

This revision is now accepted and ready to land.Mar 3 2022, 9:44 AM
This revision was landed with ongoing or failed builds.Mar 4 2022, 4:05 AM
This revision was automatically updated to reflect the committed changes.