[PATCH] D17516: AMDGPU: Verify subtarget specific builtins

[Matt Arsenault via cfe-commits]

arsenm added inline comments.

================
Comment at: lib/Basic/Targets.cpp:2059-2063
@@ +2058,7 @@
+
+  if (Has16BitInsts)
+    Features["16-bit-insts"] = true;
+
+  if (hasSMemRealTime)
+    Features["s-memrealtime"] = true;
+
----------------
echristo wrote:
> This is typically more of the "move the cpu checks down here" area from what you'd have above. Also you're not calling the target independent version of initFeatureMap - is that done on purpose?
Not sure what you mean exactly by "move the cpu checks down here". Do you mean setting all of the feature has* member variables should be moved out of the AMDGPUTargetInfo constructor + setCPU into here?

That was not done on purpose. I'm pretty confused about what all of these functions are for. I kind of expected all of this to automatically work from the target's set of defined features known from the backend. There seem to be a set of functions for manually parsing user specified features on a function and for those implied by the subtarget. Why is initFeatureMap separate from handleTargetFeatures? initFeatureMap seems to be almost the same thing with the addition of the CPU features. It's not clear to me what the difference between hasFeature and validateCpuSupports is supposed to mean, or why setFeatureEnabled is virtual.



http://reviews.llvm.org/D17516