[PATCH] D143293: AMDGPU: Use module flag to get code object version at IR level folow-up

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.cpp:807
       break;
-    case ELF::ELFABIVERSION_AMDGPU_HSA_V4:
-    case ELF::ELFABIVERSION_AMDGPU_HSA_V5:
+    case 4:
+    case 5:
----------------
cfang wrote:
> cfang wrote:
> > arsenm wrote:
> > > Going from enum names to magic constants is a regression 
> > But now we actually switch on CodeObjectVersion which is an unsigned integer, so we have to case on integers like 2, 3, 4
> > Previous we switch on getHsaAbiVersion, which is an enum value.
> > 
> > In short, I am thinking we can no longer case on enum values now.
> > But now we actually switch on CodeObjectVersion which is an unsigned integer, so we have to case on integers like 2, 3, 4
> > Previous we switch on getHsaAbiVersion, which is an enum value.
> > 
> > In short, I am thinking we can no longer case on enum values now.
> 
> Hi, above is my explanation of using the numbers 2, 3, 4, 5 for the cases the code object version cases. Please advice we should we do here if this is not appropriate?  re-define CodeObjectVersion to a different type?  Thanks.  
I don’t understand why removed the enum cases or need to fix up the values , at worst you need a cast. It would be better to use an enum for it everywhere, it would just be a larger cosmetic change 


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

https://reviews.llvm.org/D143293