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

[hfinkel at anl.gov]

REPOSITORY
  rL LLVM

================
Comment at: lib/Basic/Targets.cpp:723
@@ -722,1 +722,3 @@
   bool HasP8Crypto;
+  bool IsPwr7Up;
+  bool IsPwr8Up;
----------------
We don't need the 'Up' in the names here; I think we can generally assume that newer cores will provide backward compatibility, and only when they don't would we need some kind of HasP8Only feature. 

================
Comment at: lib/Basic/Targets.cpp:725
@@ -723,1 +724,3 @@
+  bool IsPwr8Up;
+  bool Is64Bit;
 
----------------
Please remove this. We can get at the current pointer size from generic code.

================
Comment at: lib/Basic/Targets.cpp:1217
@@ -1204,1 +1216,3 @@
+    .Case("IsISA207Up", IsPwr8Up)
+    .Case("Is64Bit", Is64Bit)
     .Default(false);
----------------
nemanjai wrote:
> nemanjai wrote:
> > 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.
> OK, as it turns out, this seems to largely be ignored by other targets (and we do not seem to have any builtins implemented that are available on 64-bit only.
> I personally feel that diagnosing this is better than just letting the back end crash.
> Example from Arm:
> 
> ```
> $ cat t.c 
> void test_clrex() {
>   __builtin_arm_clrex();
> }
> $ clang t.c --target=arm-apple-ios7.0 -c
> fatal error: error in backend: Cannot select: intrinsic %llvm.arm.clrex
> clang: error: clang frontend command failed with exit code 70 (use -v to see invocation)
> clang version 3.7.0 (trunk 231831) (llvm/trunk 231828:231843M)
> Target: arm-apple-ios7.0
> Thread model: posix
> clang: note: diagnostic msg: PLEASE submit a bug report to http://llvm.org/bugs/ and include the crash backtrace, preprocessed source, and associated run script.
> clang: note: diagnostic msg: 
> ********************
> 
> PLEASE ATTACH THE FOLLOWING FILES TO THE BUG REPORT:
> Preprocessed source(s) and associated run script(s) are located at:
> clang: note: diagnostic msg: /tmp/t-4a719a.c
> clang: note: diagnostic msg: /tmp/t-4a719a.sh
> clang: note: diagnostic msg: 
> 
> ********************
> 
> ```
> Similar example with this changeset:
> 
> ```
> $ cat a.c 
> long long test_divde(void) {
>   return __builtin_divde(74LL, 32LL);
> }
> $ clang a.c -c --target=powerpc-unknown-unknown -mcpu=pwr7
> a.c:2:10: error: This builtin is only available on 64-bit targets
>   return __builtin_divde(74LL, 32LL);
>          ^
> 1 error generated.
> 
> ```
> I personally feel that a diagnostic identifying the problem is more helpful to the user than a message that the back end crashed and suggestion to open a bug report.
I agree, we should generate an error in the frontend instead of just crashing in the backend.


================
Comment at: lib/CodeGen/CGBuiltin.cpp:6390
@@ +6389,3 @@
+                        BuiltinID == PPC::BI__builtin_bpermd;
+    if(!getTarget().hasFeature("IsPwr7Up")) {
+      CGM.Error(E->getExprLoc(),
----------------
nemanjai wrote:
> 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.
I have a weak preference for keeping the cpu -> feature logic localized to the target code, so I think this general scheme is good.

http://reviews.llvm.org/D8398

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