[PATCH] D119216: [AMDGPU] replace hostcall module flag with function attribute

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUAttributor.cpp:198
+    const auto *STI = TM.getMCSubtargetInfo();
+    return llvm::AMDGPU::getHostcallImplicitArgPosition(STI);
+  }
----------------
arsenm wrote:
> sameerds wrote:
> > arsenm wrote:
> > > The ABI should not be a property of the subtarget, and the global subtarget shouldn't be used
> > I don't understand what the objection is. Existing functions that check the ABI version clearly do so by accessing the subtarget. I am merely following existing practice.
> > 
> > I have now rearranged the code a bit. Maybe this works? To be honest, I am not very familiar with how ABI information is tracked. I will heartily welcome any advice on how to retrieve the ABI version and then determine the location of the hostcall implicit arg.
> This is still depending on the global subtarget. There's no reason isHsaAbiVersion* should depend on the subtarget. All it's doing is checking global properties
Fundamentally the ABI is/needs to be a module level property. The subtarget is a function level property, and the subtarget read from the global target machine has been deprecated for years. getHsaABIVersion isn't actually reading anything from the subtarget, and is only using it to query the triple. Really getHsaAbiVersion should be passed the module, which could query the triple/module flag/compiler flag, none of which are subtarget properties


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D119216/new/

https://reviews.llvm.org/D119216