Hi Nemanja,
Just a few comments inline.
REPOSITORY
rL LLVM
================
Comment at: lib/Basic/Targets.cpp:1213
@@ -1203,1 +1212,3 @@
.Case("crypto", HasP8Crypto)
+ .Case("IsPwr7Up", IsPwr7Up)
+ .Case("IsISA206Up", IsPwr7Up)
----------------
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.
================
Comment at: lib/Basic/Targets.cpp:1217
@@ -1204,1 +1216,3 @@
+ .Case("IsISA207Up", IsPwr8Up)
+ .Case("Is64Bit", Is64Bit)
.Default(false);
----------------
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.
================
Comment at: lib/CodeGen/CGBuiltin.cpp:6390
@@ +6389,3 @@
+ BuiltinID == PPC::BI__builtin_bpermd;
+ if(!getTarget().hasFeature("IsPwr7Up")) {
+ CGM.Error(E->getExprLoc(),
----------------
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.
http://reviews.llvm.org/D8398
EMAIL PREFERENCES
http://reviews.llvm.org/settings/panel/emailpreferences/
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
