This is an archive of the discontinued LLVM Phabricator instance.

AMDGPU: Emit note directive for HSA even if there are no functions
ClosedPublic

Authored by tstellarAMD on Jan 8 2016, 1:02 PM.

Diff Detail

Repository
rL LLVM

Event Timeline

tstellarAMD retitled this revision from to AMDGPU: Emit note directive for HSA even if there are no functions.
tstellarAMD updated this object.
tstellarAMD added reviewers: arsenm, echristo.
tstellarAMD added a subscriber: llvm-commits.
tstellarAMD planned changes to this revision.Jan 8 2016, 1:42 PM

Spoke with echristo an IRC, and I'm going to implement this a better way.

Move getISAVersion() into the TargetMachine.

echristo added inline comments.Jan 8 2016, 4:38 PM
lib/Target/AMDGPU/AMDGPUTargetMachine.h
90 ↗(On Diff #44389)

This is pretty gross. Why not just move the variable to the TargetMachine or have a function that's part of the target machine that looks up ISA version based on... whatever it is it's based on? :)

tstellarAMD added inline comments.Jan 8 2016, 5:00 PM
lib/Target/AMDGPU/AMDGPUTargetMachine.h
90 ↗(On Diff #44389)

ISA version is based on the CPU string. The reason to do it this way is the logic mapping CPU strings to IsaVersion already exists in the .TD files and I didn't want to duplicate the logic here, but if implementing it this way is a problem then I can duplicate the logic here, it should be a problem. Do you have a preference?

Construct an MCSubtargetInfo object in the AsmPrinter so we can fetch
the feature bits.

echristo accepted this revision.Jan 11 2016, 1:57 PM
echristo edited edge metadata.

This is fine. You should probably clean up getIsaVersion. It appears to be just used as a tag in the object so it doesn't really need a subtarget feature, you could just have a static version that returns a number.

-eric

This revision is now accepted and ready to land.Jan 11 2016, 1:57 PM
This revision was automatically updated to reflect the committed changes.