[PATCH] D54301: [AMDGPU] Derive GCNSubtarget from MF to get overridden target features

Tue Nov 13 08:53:28 PST 2018

dstuttard added a comment.

In https://reviews.llvm.org/D54301#1296006, @scott.linder wrote:

> I don't quite understand what distinguishes the two versions of MCSubtargetInfo (getSTI() vs STM), but it appears a patch from Konstantin earlier this year changed this specific instance from `STM.getFeatureBits()` to `getSTI()`. Changing this back LGTM but I don't know what the rationale was in the first place, if it wasn't a typo, so I don't want to sign off without asking.

My assumption is that it was neater to use the getSTI() member function provided - but that get the GCNSubtarget from the TargetMachine rather than MachineFunction - in most cases I guess this doesn't matter - but in our case the MF has got subtarget features enabled that the TM version doesn't. I wonder if the getSTI should be changed to use the MF version and all references to STM moved over to use getSTI().

Like you I'm not 100% sure there isn't a reason to use the TargetMachine derived version - so I guess we need Konstantin to comment.

Repository:
  rL LLVM

https://reviews.llvm.org/D54301