This is an archive of the discontinued LLVM Phabricator instance.

[AMDGPU] predicate and feature refactoring
ClosedPublic

Authored by rampitec on Apr 4 2019, 4:30 PM.

Details

Summary

We have done some predicate and feature refactoring lately but did not upstream it. This is to sync.

Diff Detail

Repository
rL LLVM

Event Timeline

rampitec created this revision.Apr 4 2019, 4:30 PM
rampitec updated this revision to Diff 193911.Apr 5 2019, 10:56 AM

Also renamed decoder namespaces.

kzhuravl accepted this revision.Apr 5 2019, 10:59 AM

LGTM. Minor comment which does not affect the LGTM! Thanks for doing this.

lib/Target/AMDGPU/AMDGPU.td
714–716 ↗(On Diff #193911)

Should this be renamed at some point too?

This revision is now accepted and ready to land.Apr 5 2019, 10:59 AM
rampitec marked an inline comment as done.Apr 5 2019, 11:01 AM
rampitec added inline comments.
lib/Target/AMDGPU/AMDGPU.td
714–716 ↗(On Diff #193911)

Yes, but I want this to go from upstream and not the other way around. So a separate change.

kzhuravl added inline comments.Apr 5 2019, 11:03 AM
lib/Target/AMDGPU/AMDGPU.td
714–716 ↗(On Diff #193911)

Sounds good, thanks.

arsenm added inline comments.Apr 5 2019, 11:10 AM
lib/Target/AMDGPU/AMDGPU.td
146–150 ↗(On Diff #193911)

This is a compatibility breaking change, but we haven't been trying too hard to not break bitcode

rampitec marked an inline comment as done.Apr 5 2019, 11:13 AM
rampitec added inline comments.
lib/Target/AMDGPU/AMDGPU.td
146–150 ↗(On Diff #193911)

We managed to have two features at the same time, vi-insts and gfx8-insts, so compatibility is already compromised. This will just combine them from this point on.

This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptApr 5 2019, 11:23 AM

@arsenm - could write a bitcode upgrade path to deal with the breakage