[PATCH] Add Clang support for remaining integer divide and permute instructions from ISA 2.06

[Nemanja Ivanovic]

REPOSITORY
  rL LLVM

================
Comment at: lib/Basic/Targets.cpp:1213
@@ -1203,1 +1212,3 @@
     .Case("crypto", HasP8Crypto)
+    .Case("IsPwr7Up", IsPwr7Up)
+    .Case("IsISA206Up", IsPwr7Up)
----------------
wschmidt wrote:
> These strings should not be capitalized, to maintain consistency.  As in the other review, I'm not excited about the name.
> 
> I really question whether these should be features.  Doesn't that imply that these become things that can be controlled independently as -mattr=+/- for llc?  I don't think we want that.
Good point about the capitalization. I just used the same names as the bool variables. I can change this.

Regarding the second point, I was careful not to expose these features outside of the target info. They will not become features that have a corresponding Clang option nor will they give the user any control over turning these on/off. This just provides a string-based query for the data members that I added. Essentially, this is a unified "getter" that provides a name-based query.
All that being said, I do see your point. Everything else in this string->bool map is an actual feature that a user has control over. These new ones deviate from that. I am open to suggestions on how to achieve what we are trying to achieve here (namely, control over exposing builtins for non-optional instructions).

================
Comment at: lib/Basic/Targets.cpp:1217
@@ -1204,1 +1216,3 @@
+    .Case("IsISA207Up", IsPwr8Up)
+    .Case("Is64Bit", Is64Bit)
     .Default(false);
----------------
wschmidt wrote:
> mmm, even if we keep this approach, I'm not sure about the Is64Bit one.  Surely these aren't the only built-ins that only work for 64-bit.  If you're going to add this it needs to be complete and consistent, which is far more than what you're going for here.  My inclination is to leave it out and consider whether a full patch for this is sensible down the road.
I will investigate whether other 64-bit only builtins provide any diagnostics if used in code that is compiled at 32-bit. Thanks for bringing this up.

================
Comment at: lib/CodeGen/CGBuiltin.cpp:6390
@@ +6389,3 @@
+                        BuiltinID == PPC::BI__builtin_bpermd;
+    if(!getTarget().hasFeature("IsPwr7Up")) {
+      CGM.Error(E->getExprLoc(),
----------------
wschmidt wrote:
> I'd prefer that the conditions you've encoded as features just be tested here, rather than using the feature mechanism, but this is a call for people with more experience with this than me.
I am certainly not opposed to this. Doing so would negate the need for the "fake features" from above. However, the reason for doing it this way was in anticipation of any future CPU's that are a superset of Power7...
Say we implement 5 builtins that are only available on Power7. Then another 17 that are available on Power8. We then get CPU X which is a superset of both. We would need to track down all the places where we check for the correct CPU. Using this approach puts this check in one place.
However, all this is just to explain why I did it this way. If you (or other interested parties) feel that I shouldn't, I'd be happy to change it.

http://reviews.llvm.org/D8398

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/