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

hfinkel at anl.gov hfinkel at anl.gov
Mon Mar 23 13:20:48 PDT 2015


REPOSITORY
  rL LLVM

================
Comment at: lib/Basic/Targets.cpp:1213
@@ -1203,1 +1212,3 @@
     .Case("crypto", HasP8Crypto)
+    .Case("IsPwr7Up", IsPwr7Up)
+    .Case("IsISA206Up", IsPwr7Up)
----------------
nemanjai wrote:
> 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).
Also, please keep the existing naming convention for these (lowercase, etc.), and we don't need all of the aliases. How about just adding features for 'isa-2.06', 'isa-2.07', etc.?

http://reviews.llvm.org/D8398

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






More information about the cfe-commits mailing list