This is an archive of the discontinued LLVM Phabricator instance.

AMDGPU: Cleanup most of the macros
ClosedPublic

Authored by kzhuravl on Aug 16 2017, 11:40 AM.

Details

Summary
  • Insert AMD macro
  • Insert AMDGPU macro
  • Insert devicename macro
  • Add missing tests for arch macros

Diff Detail

Repository
rL LLVM

Event Timeline

kzhuravl created this revision.Aug 16 2017, 11:40 AM

Do we need this for r600?

t-tye added inline comments.Aug 16 2017, 4:19 PM
lib/Basic/Targets/AMDGPU.cpp
364–367 ↗(On Diff #111390)

Should this be the following since extra macros could be after it in the future:

if (!GPUName.empty())
  Builder.defineMacro(Twine("__") + Twine(GPUName) + Twine("__"));

Should we only be defining macros using the canonical target name (the one in column 1 of https://llvm.org/docs/AMDGPUUsage.html#processors) rather than the one specified on the command line?

kzhuravl updated this revision to Diff 112761.Aug 25 2017, 4:22 PM
kzhuravl marked an inline comment as done.
kzhuravl retitled this revision from AMDGPU: Insert __devicename__ macros to AMDGPU: Cleanup most of the macros.
kzhuravl edited the summary of this revision. (Show Details)

fast fmaf macro will be in the follow up patch.

b-sumner added inline comments.Aug 28 2017, 9:52 AM
lib/Basic/Targets/AMDGPU.cpp
362 ↗(On Diff #112761)

At the meeting we discussed defining every alias of the given GPU, rather than only the mcpu name. Were we going to proceed with that?

kzhuravl planned changes to this revision.Oct 17 2017, 2:48 PM

Need to only define canonical names.

kzhuravl updated this revision to Diff 133569.Feb 9 2018, 1:01 AM

Define only canonical names.

t-tye requested changes to this revision.Feb 14 2018, 2:01 PM
t-tye added inline comments.
lib/Basic/Targets/AMDGPU.cpp
336 ↗(On Diff #133569)

Subsequent discussion decided to drop this macro.

362 ↗(On Diff #112761)

Subsequent discussion settled on only defining the canonical names. If it was useful to also have the alternative names a header file could be provided that defines those names in response to the canonical names being defined.

This revision now requires changes to proceed.Feb 14 2018, 2:01 PM
kzhuravl updated this revision to Diff 134335.Feb 14 2018, 3:57 PM
kzhuravl edited the summary of this revision. (Show Details)

Address review feedback.

kzhuravl marked 3 inline comments as done.Feb 14 2018, 3:57 PM
t-tye accepted this revision.Feb 14 2018, 4:00 PM

LGTM

This revision is now accepted and ready to land.Feb 14 2018, 4:00 PM
This revision was automatically updated to reflect the committed changes.