[PATCH] D66199: [docs] loop pragmas

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer marked an inline comment as done.
SjoerdMeijer added a comment.

> Since this is user documentation, we should only add it here once it is true.

Yep, good point, I also wanted to capture our discussions on the list. But yes, 
let's hold this back until we're ready with the pragmas.




Comment at: docs/LanguageExtensions.rst:3068-3069
 
+There are loop hints that control transformations (e.g. vectorization, loop
+unrolling) and there loop hints that set transformation options (e.g.
+``vectorize_width``, ``unroll_count``).  Pragmas setting transformation options

Meinersbur wrote:
> `vectorize_width` also "controls" a transformation. Is it that in our 
> interpretation, `vectorize(enable)` overrides the profitability heuristic by 
> taking out the "do not apply at all" option without setting any other option?
I think I need to think a little bit more about this:

> Is it that in our interpretation, vectorize(enable) overrides the 
> profitability heuristic by taking out the "do not apply at all" option 
> without setting any other option?

but I would answer this with "yes". But just checking, what exactly do you mean 
with "do not apply at all option"?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66199/new/

https://reviews.llvm.org/D66199



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D64564: Loop pragma parsing. NFC.

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer updated this revision to Diff 215162.
SjoerdMeijer added a comment.

thanks for the suggestions; comments addressed.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64564/new/

https://reviews.llvm.org/D64564

Files:
  clang/lib/Parse/ParsePragma.cpp


Index: clang/lib/Parse/ParsePragma.cpp
===
--- clang/lib/Parse/ParsePragma.cpp
+++ clang/lib/Parse/ParsePragma.cpp
@@ -1006,18 +1006,14 @@
 } // end anonymous namespace
 
 static std::string PragmaLoopHintString(Token PragmaName, Token Option) {
-  std::string PragmaString;
-  if (PragmaName.getIdentifierInfo()->getName() == "loop") {
-PragmaString = "clang loop ";
-PragmaString += Option.getIdentifierInfo()->getName();
-  } else if (PragmaName.getIdentifierInfo()->getName() == "unroll_and_jam") {
-PragmaString = "unroll_and_jam";
-  } else {
-assert(PragmaName.getIdentifierInfo()->getName() == "unroll" &&
-   "Unexpected pragma name");
-PragmaString = "unroll";
-  }
-  return PragmaString;
+  StringRef Str = PragmaName.getIdentifierInfo()->getName();
+  std::string ClangLoopStr = (Twine("clang loop ") + Str).str();
+  std::string Result = llvm::StringSwitch(Str)
+ .Case("loop", ClangLoopStr)
+ .Case("unroll_and_jam", Str)
+ .Case("unroll", Str)
+ .Default("");
+  return Result;
 }
 
 bool Parser::HandlePragmaLoopHint(LoopHint ) {
@@ -1041,12 +1037,12 @@
 
   // Return a valid hint if pragma unroll or nounroll were specified
   // without an argument.
-  bool PragmaUnroll = PragmaNameInfo->getName() == "unroll";
-  bool PragmaNoUnroll = PragmaNameInfo->getName() == "nounroll";
-  bool PragmaUnrollAndJam = PragmaNameInfo->getName() == "unroll_and_jam";
-  bool PragmaNoUnrollAndJam = PragmaNameInfo->getName() == "nounroll_and_jam";
-  if (Toks.empty() && (PragmaUnroll || PragmaNoUnroll || PragmaUnrollAndJam ||
-   PragmaNoUnrollAndJam)) {
+  auto IsLoopHint = llvm::StringSwitch(PragmaNameInfo->getName())
+.Cases("unroll", "nounroll", "unroll_and_jam",
+   "nounroll_and_jam", true)
+.Default(false);
+
+  if (Toks.empty() && IsLoopHint) {
 ConsumeAnnotationToken();
 Hint.Range = Info->PragmaName.getLocation();
 return true;


Index: clang/lib/Parse/ParsePragma.cpp
===
--- clang/lib/Parse/ParsePragma.cpp
+++ clang/lib/Parse/ParsePragma.cpp
@@ -1006,18 +1006,14 @@
 } // end anonymous namespace
 
 static std::string PragmaLoopHintString(Token PragmaName, Token Option) {
-  std::string PragmaString;
-  if (PragmaName.getIdentifierInfo()->getName() == "loop") {
-PragmaString = "clang loop ";
-PragmaString += Option.getIdentifierInfo()->getName();
-  } else if (PragmaName.getIdentifierInfo()->getName() == "unroll_and_jam") {
-PragmaString = "unroll_and_jam";
-  } else {
-assert(PragmaName.getIdentifierInfo()->getName() == "unroll" &&
-   "Unexpected pragma name");
-PragmaString = "unroll";
-  }
-  return PragmaString;
+  StringRef Str = PragmaName.getIdentifierInfo()->getName();
+  std::string ClangLoopStr = (Twine("clang loop ") + Str).str();
+  std::string Result = llvm::StringSwitch(Str)
+ .Case("loop", ClangLoopStr)
+ .Case("unroll_and_jam", Str)
+ .Case("unroll", Str)
+ .Default("");
+  return Result;
 }
 
 bool Parser::HandlePragmaLoopHint(LoopHint ) {
@@ -1041,12 +1037,12 @@
 
   // Return a valid hint if pragma unroll or nounroll were specified
   // without an argument.
-  bool PragmaUnroll = PragmaNameInfo->getName() == "unroll";
-  bool PragmaNoUnroll = PragmaNameInfo->getName() == "nounroll";
-  bool PragmaUnrollAndJam = PragmaNameInfo->getName() == "unroll_and_jam";
-  bool PragmaNoUnrollAndJam = PragmaNameInfo->getName() == "nounroll_and_jam";
-  if (Toks.empty() && (PragmaUnroll || PragmaNoUnroll || PragmaUnrollAndJam ||
-   PragmaNoUnrollAndJam)) {
+  auto IsLoopHint = llvm::StringSwitch(PragmaNameInfo->getName())
+.Cases("unroll", "nounroll", "unroll_and_jam",
+   "nounroll_and_jam", true)
+.Default(false);
+
+  if (Toks.empty() && IsLoopHint) {
 ConsumeAnnotationToken();
 Hint.Range = Info->PragmaName.getLocation();
 return true;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D66199: [docs] loop pragmas

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer created this revision.
SjoerdMeijer added reviewers: fhahn, Meinersbur, dorit, hsaito.

Following our discussion on the cfe dev list
(http://lists.llvm.org/pipermail/cfe-dev/2019-August/063054.html), I have added
a paragraph that is explicit about transformation options implying
the corresponding transformation.


https://reviews.llvm.org/D66199

Files:
  docs/LanguageExtensions.rst


Index: docs/LanguageExtensions.rst
===
--- docs/LanguageExtensions.rst
+++ docs/LanguageExtensions.rst
@@ -3065,6 +3065,14 @@
 distribution. Loop hints can be specified before any loop and will be ignored 
if
 the optimization is not safe to apply.
 
+There are loop hints that control transformations (e.g. vectorization, loop
+unrolling) and there loop hints that set transformation options (e.g.
+``vectorize_width``, ``unroll_count``).  Pragmas setting transformation options
+imply the transformation is enabled, as if it was enabled via the corresponding
+transformation pragma (e.g. ``vectorize(enable)``). If the transformation is
+disabled  (e.g. ``vectorize(disable)``), that takes precedence over
+transformations option pragmas implying that transformation.
+
 Vectorization, Interleaving, and Predication
 
 


Index: docs/LanguageExtensions.rst
===
--- docs/LanguageExtensions.rst
+++ docs/LanguageExtensions.rst
@@ -3065,6 +3065,14 @@
 distribution. Loop hints can be specified before any loop and will be ignored if
 the optimization is not safe to apply.
 
+There are loop hints that control transformations (e.g. vectorization, loop
+unrolling) and there loop hints that set transformation options (e.g.
+``vectorize_width``, ``unroll_count``).  Pragmas setting transformation options
+imply the transformation is enabled, as if it was enabled via the corresponding
+transformation pragma (e.g. ``vectorize(enable)``). If the transformation is
+disabled  (e.g. ``vectorize(disable)``), that takes precedence over
+transformations option pragmas implying that transformation.
+
 Vectorization, Interleaving, and Predication
 
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D64564: Loop pragma parsing. NFC.

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer added a comment.

Sorry, should have done my std::string, StringRef, and Twine homework a lot 
better!

Thanks for your help and suggestions, will fix this.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64564/new/

https://reviews.llvm.org/D64564



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D65776: [Clang] Pragma vectorize_predicate implies vectorize

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer added a comment.

Thanks for your help! And I will wait a few days.

After this, I will look at that PR, will have a look at diagnostics, and then 
at the LLVM side of things.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65776/new/

https://reviews.llvm.org/D65776



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D64564: Loop pragma parsing. NFC.

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer added a comment.

this is not important at all, but might be nice clean up, so friendly ping :-)


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64564/new/

https://reviews.llvm.org/D64564



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D65776: [Clang] Pragma vectorize_predicate implies vectorize

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer added a comment.

thanks @Meinersbur and @fhahn  for the discussions on this, also on the dev 
list.
It looks like we are happy with this behaviour?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65776/new/

https://reviews.llvm.org/D65776



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D65776: [Clang] Pragma vectorize_predicate implies vectorize

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer added a comment.

> I do not know whether/how "setting a transformation option implicitly enables 
> the transformation" should be implemented, maybe we should discuss this.

Yep, agreed. I've sent a mail to the dev list:
http://lists.llvm.org/pipermail/cfe-dev/2019-August/063054.html


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65776/new/

https://reviews.llvm.org/D65776



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D65776: [Clang] Pragma vectorize_predicate implies vectorize

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer added a comment.

Hi Florian, thanks for your input!

> IMO it would make sense to have the more specific pragmas imply 
> vectorize(enable) here (or update the docs accordingly).

Yep, fully agree with that, as I also wrote in my previous comment. And thanks 
for digging up that PR. I think that supports this case and that the proposed 
behaviour in this patch is what one would expect. If we agree on that, I 
wouldn't mind fixing that PR too as that seems low hanging-fruit to me.

And to be honest, at the moment I have no clue what the meaning is, or 
could/should be, when a more specific pragma like vector_width or 
vector_predicate is set, without also setting/implying vectorize(enable). I 
need to look into that...


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65776/new/

https://reviews.llvm.org/D65776



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D65776: [Clang] Pragma vectorize_predicate implies vectorize

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer added a comment.

Thanks for pointing this all out!

I am not entirely sure yet what to think about all this as I am new to the loop 
pragma business, but I think it looks inconsistent to me!

I think I find `allowReordering()` a little bit ugly, because it is also 
checking `getWidth() > 1`, and I am probably expecting the allowReordering 
decision to be based on 1 thing (the Force). But in the current implementation 
that probably makes sense though if `vectorizer_width` doesn't set 
`vectorizer.enable` (again, which is not what I would expect). Thus, it makes 
sense to me that `LoopVectorizationCostModel::selectVectorizationFactor()` only 
checks `getForce()`

> Mmmh, I would have expected this to work the same way as `vectorize_width`.

So I don't see at this moment how it could work in the same way. And by working 
in the same way I think you mean adding `vectorize_predicate` checks to 
different places where `vectorize_width` is also checked, but that doesn't 
sound ideal to me.

Could a conclusion be that I was lucky? Lucky, because without all this 
knowledge that I have now, this patch looks to do what we would expect?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65776/new/

https://reviews.llvm.org/D65776



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D65776: [Clang] Pragma vectorize_predicate implies vectorize

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer added a comment.

Forgot that this requires a doc change too. Will add that once we're happy with 
the proposed behaviour.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65776/new/

https://reviews.llvm.org/D65776



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D65776: [Clang] Pragma vectorize_predicate implies vectorize

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer created this revision.
SjoerdMeijer added reviewers: Meinersbur, hsaito, fhahn.

New pragma "vectorize_predicate(enable)" now implies "vectorize(enable)",
and it is ignored when vectorization is disabled with e.g.
"vectorize(disable) vectorize_predicate(enable)".


https://reviews.llvm.org/D65776

Files:
  clang/lib/CodeGen/CGLoopInfo.cpp
  clang/test/CodeGenCXX/pragma-loop-predicate.cpp


Index: clang/test/CodeGenCXX/pragma-loop-predicate.cpp
===
--- clang/test/CodeGenCXX/pragma-loop-predicate.cpp
+++ clang/test/CodeGenCXX/pragma-loop-predicate.cpp
@@ -27,9 +27,50 @@
 List[i] = i * 2;
 }
 
+// vectorize_predicate(enable) implies vectorize(enable)
+void test3(int *List, int Length) {
+// CHECK-LABEL: @{{.*}}test3{{.*}}(
+// CHECK:   br label {{.*}}, !llvm.loop ![[LOOP3:.*]]
+
+  #pragma clang loop vectorize_predicate(enable)
+  for (int i = 0; i < Length; i++)
+List[i] = i * 2;
+}
+
+// Check that disabling vectorization means a vectorization width of 1, and
+// also that vectorization_predicate isn't enabled.
+void test4(int *List, int Length) {
+// CHECK-LABEL: @{{.*}}test4{{.*}}(
+// CHECK: br label {{.*}}, !llvm.loop ![[LOOP4:.*]]
+
+  #pragma clang loop vectorize(disable)
+  for (int i = 0; i < Length; i++)
+List[i] = i * 2;
+}
+
+// Check that vectorize and vectorize_predicate are disabled.
+void test5(int *List, int Length) {
+// CHECK-LABEL: @{{.*}}test5{{.*}}(
+// CHECK: br label {{.*}}, !llvm.loop ![[LOOP5:.*]]
+
+  #pragma clang loop vectorize(disable) vectorize_predicate(enable)
+  for (int i = 0; i < Length; i++)
+List[i] = i * 2;
+}
+
+
 // CHECK:  ![[LOOP0]] = distinct !{![[LOOP0]], !3}
 // CHECK-NEXT: !3 = !{!"llvm.loop.vectorize.enable", i1 true}
+
 // CHECK-NEXT: ![[LOOP1]] = distinct !{![[LOOP1]], !5, !3}
 // CHECK-NEXT: !5 = !{!"llvm.loop.vectorize.predicate.enable", i1 true}
+
 // CHECK-NEXT: ![[LOOP2]] = distinct !{![[LOOP2]], !7, !3}
 // CHECK-NEXT: !7 = !{!"llvm.loop.vectorize.predicate.enable", i1 false}
+
+// CHECK-NEXT: ![[LOOP3]] = distinct !{![[LOOP3]], !5, !3}
+
+// CHECK-NEXT: ![[LOOP4]] = distinct !{![[LOOP4]], !10}
+// CHECK-NEXT: !10 = !{!"llvm.loop.vectorize.width", i32 1}
+
+// CHECK-NEXT: ![[LOOP5]] = distinct !{![[LOOP5]], !10}
Index: clang/lib/CodeGen/CGLoopInfo.cpp
===
--- clang/lib/CodeGen/CGLoopInfo.cpp
+++ clang/lib/CodeGen/CGLoopInfo.cpp
@@ -253,12 +253,18 @@
   Args.append(LoopProperties.begin(), LoopProperties.end());
 
   // Setting vectorize.predicate
-  if (Attrs.VectorizePredicateEnable != LoopAttributes::Unspecified) {
+  bool IsVectorPredicateEnabled = false;
+  if (Attrs.VectorizePredicateEnable != LoopAttributes::Unspecified &&
+  Attrs.VectorizeEnable != LoopAttributes::Disable &&
+  Attrs.VectorizeWidth < 1) {
+
+IsVectorPredicateEnabled =
+(Attrs.VectorizePredicateEnable == LoopAttributes::Enable);
+
 Metadata *Vals[] = {
 MDString::get(Ctx, "llvm.loop.vectorize.predicate.enable"),
-ConstantAsMetadata::get(ConstantInt::get(
-llvm::Type::getInt1Ty(Ctx),
-(Attrs.VectorizePredicateEnable == LoopAttributes::Enable)))};
+ConstantAsMetadata::get(ConstantInt::get(llvm::Type::getInt1Ty(Ctx),
+ IsVectorPredicateEnabled))};
 Args.push_back(MDNode::get(Ctx, Vals));
   }
 
@@ -281,12 +287,15 @@
   }
 
   // Setting vectorize.enable
-  if (Attrs.VectorizeEnable != LoopAttributes::Unspecified) {
+  if (Attrs.VectorizeEnable != LoopAttributes::Unspecified ||
+  IsVectorPredicateEnabled) {
 Metadata *Vals[] = {
 MDString::get(Ctx, "llvm.loop.vectorize.enable"),
 ConstantAsMetadata::get(ConstantInt::get(
 llvm::Type::getInt1Ty(Ctx),
-(Attrs.VectorizeEnable == LoopAttributes::Enable)))};
+IsVectorPredicateEnabled
+? true
+: (Attrs.VectorizeEnable == LoopAttributes::Enable)))};
 Args.push_back(MDNode::get(Ctx, Vals));
   }
 


Index: clang/test/CodeGenCXX/pragma-loop-predicate.cpp
===
--- clang/test/CodeGenCXX/pragma-loop-predicate.cpp
+++ clang/test/CodeGenCXX/pragma-loop-predicate.cpp
@@ -27,9 +27,50 @@
 List[i] = i * 2;
 }
 
+// vectorize_predicate(enable) implies vectorize(enable)
+void test3(int *List, int Length) {
+// CHECK-LABEL: @{{.*}}test3{{.*}}(
+// CHECK:   br label {{.*}}, !llvm.loop ![[LOOP3:.*]]
+
+  #pragma clang loop vectorize_predicate(enable)
+  for (int i = 0; i < Length; i++)
+List[i] = i * 2;
+}
+
+// Check that disabling vectorization means a vectorization width of 1, and
+// also that vectorization_predicate isn't enabled.
+void test4(int *List, int Length) {
+// CHECK-LABEL: @{{.*}}test4{{.*}}(
+// CHECK: br label {{.*}}, !llvm.loop ![[LOOP4:.*]]
+
+  #pragma clang loop vectorize(disable)
+ 

[PATCH] D64564: Loop pragma parsing. NFC.

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer updated this revision to Diff 212878.
SjoerdMeijer added a comment.

Fixed the string problems.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64564/new/

https://reviews.llvm.org/D64564

Files:
  clang/lib/Parse/ParsePragma.cpp


Index: clang/lib/Parse/ParsePragma.cpp
===
--- clang/lib/Parse/ParsePragma.cpp
+++ clang/lib/Parse/ParsePragma.cpp
@@ -1006,18 +1006,15 @@
 } // end anonymous namespace
 
 static std::string PragmaLoopHintString(Token PragmaName, Token Option) {
-  std::string PragmaString;
-  if (PragmaName.getIdentifierInfo()->getName() == "loop") {
-PragmaString = "clang loop ";
-PragmaString += Option.getIdentifierInfo()->getName();
-  } else if (PragmaName.getIdentifierInfo()->getName() == "unroll_and_jam") {
-PragmaString = "unroll_and_jam";
-  } else {
-assert(PragmaName.getIdentifierInfo()->getName() == "unroll" &&
-   "Unexpected pragma name");
-PragmaString = "unroll";
-  }
-  return PragmaString;
+  StringRef Str = PragmaName.getIdentifierInfo()->getName();
+  StringRef ClangLoopStr = "clang loop " + Str.str();
+  Str = llvm::StringSwitch(Str)
+   .Case("loop", ClangLoopStr)
+   .Case("unroll_and_jam", Str)
+   .Case("unroll", Str)
+   .Default("");
+  assert(Str.size() && "Unexpected pragma name");
+  return std::string(Str);
 }
 
 bool Parser::HandlePragmaLoopHint(LoopHint ) {
@@ -1041,12 +1038,12 @@
 
   // Return a valid hint if pragma unroll or nounroll were specified
   // without an argument.
-  bool PragmaUnroll = PragmaNameInfo->getName() == "unroll";
-  bool PragmaNoUnroll = PragmaNameInfo->getName() == "nounroll";
-  bool PragmaUnrollAndJam = PragmaNameInfo->getName() == "unroll_and_jam";
-  bool PragmaNoUnrollAndJam = PragmaNameInfo->getName() == "nounroll_and_jam";
-  if (Toks.empty() && (PragmaUnroll || PragmaNoUnroll || PragmaUnrollAndJam ||
-   PragmaNoUnrollAndJam)) {
+  auto IsLoopHint = llvm::StringSwitch(PragmaNameInfo->getName())
+.Cases("unroll", "nounroll", "unroll_and_jam",
+   "nounroll_and_jam", true)
+.Default(false);
+
+  if (Toks.empty() && IsLoopHint) {
 ConsumeAnnotationToken();
 Hint.Range = Info->PragmaName.getLocation();
 return true;


Index: clang/lib/Parse/ParsePragma.cpp
===
--- clang/lib/Parse/ParsePragma.cpp
+++ clang/lib/Parse/ParsePragma.cpp
@@ -1006,18 +1006,15 @@
 } // end anonymous namespace
 
 static std::string PragmaLoopHintString(Token PragmaName, Token Option) {
-  std::string PragmaString;
-  if (PragmaName.getIdentifierInfo()->getName() == "loop") {
-PragmaString = "clang loop ";
-PragmaString += Option.getIdentifierInfo()->getName();
-  } else if (PragmaName.getIdentifierInfo()->getName() == "unroll_and_jam") {
-PragmaString = "unroll_and_jam";
-  } else {
-assert(PragmaName.getIdentifierInfo()->getName() == "unroll" &&
-   "Unexpected pragma name");
-PragmaString = "unroll";
-  }
-  return PragmaString;
+  StringRef Str = PragmaName.getIdentifierInfo()->getName();
+  StringRef ClangLoopStr = "clang loop " + Str.str();
+  Str = llvm::StringSwitch(Str)
+   .Case("loop", ClangLoopStr)
+   .Case("unroll_and_jam", Str)
+   .Case("unroll", Str)
+   .Default("");
+  assert(Str.size() && "Unexpected pragma name");
+  return std::string(Str);
 }
 
 bool Parser::HandlePragmaLoopHint(LoopHint ) {
@@ -1041,12 +1038,12 @@
 
   // Return a valid hint if pragma unroll or nounroll were specified
   // without an argument.
-  bool PragmaUnroll = PragmaNameInfo->getName() == "unroll";
-  bool PragmaNoUnroll = PragmaNameInfo->getName() == "nounroll";
-  bool PragmaUnrollAndJam = PragmaNameInfo->getName() == "unroll_and_jam";
-  bool PragmaNoUnrollAndJam = PragmaNameInfo->getName() == "nounroll_and_jam";
-  if (Toks.empty() && (PragmaUnroll || PragmaNoUnroll || PragmaUnrollAndJam ||
-   PragmaNoUnrollAndJam)) {
+  auto IsLoopHint = llvm::StringSwitch(PragmaNameInfo->getName())
+.Cases("unroll", "nounroll", "unroll_and_jam",
+   "nounroll_and_jam", true)
+.Default(false);
+
+  if (Toks.empty() && IsLoopHint) {
 ConsumeAnnotationToken();
 Hint.Range = Info->PragmaName.getLocation();
 return true;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D64564: Loop pragma parsing. NFC.

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer marked an inline comment as done.
SjoerdMeijer added inline comments.



Comment at: lib/Parse/ParsePragma.cpp:1011
+  Str = llvm::StringSwitch(Str)
+   .Case("loop", "clang loop " + Str.str())
+   .Case("unroll_and_jam", Str)

Meinersbur wrote:
> [serious] I know I already accepted the patch, but I just noticed something:
> `"clang loop " + Str.str()` will allocate a temporary std::string, `Str` will 
> potentially point to it, then the temporary string will be released. `Str` 
> will then point to released memory and returned by this function, i.e. a 
> use-after-free.
> 
Oopsy, thanks for spotting this! Will fix this!


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64564/new/

https://reviews.llvm.org/D64564



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D64744: #pragma clang loop vectorize_predicate(enable|disable)

[Sjoerd Meijer via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL366989: [Clang] New loop pragma vectorize_predicate 
(authored by SjoerdMeijer, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D64744?vs=211096=211678#toc

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64744/new/

https://reviews.llvm.org/D64744

Files:
  cfe/trunk/docs/LanguageExtensions.rst
  cfe/trunk/include/clang/Basic/Attr.td
  cfe/trunk/include/clang/Basic/AttrDocs.td
  cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td
  cfe/trunk/lib/CodeGen/CGLoopInfo.cpp
  cfe/trunk/lib/CodeGen/CGLoopInfo.h
  cfe/trunk/lib/Parse/ParsePragma.cpp
  cfe/trunk/lib/Sema/SemaStmtAttr.cpp
  cfe/trunk/test/AST/ast-print-pragmas.cpp
  cfe/trunk/test/CodeGenCXX/pragma-loop-predicate.cpp
  cfe/trunk/test/Parser/pragma-loop.cpp
  cfe/trunk/test/Parser/pragma-unroll-and-jam.cpp
  llvm/trunk/docs/LangRef.rst

Index: llvm/trunk/docs/LangRef.rst
===
--- llvm/trunk/docs/LangRef.rst
+++ llvm/trunk/docs/LangRef.rst
@@ -5423,6 +5423,21 @@
!0 = !{!"llvm.loop.vectorize.enable", i1 0}
!1 = !{!"llvm.loop.vectorize.enable", i1 1}
 
+'``llvm.loop.vectorize.predicate.enable``' Metadata
+^^^
+
+This metadata selectively enables or disables creating predicated instructions
+for the loop, which can enable folding of the scalar epilogue loop into the
+main loop. The first operand is the string
+``llvm.loop.vectorize.predicate.enable`` and the second operand is a bit. If
+the bit operand value is 1 vectorization is enabled. A value of 0 disables
+vectorization:
+
+.. code-block:: llvm
+
+   !0 = !{!"llvm.loop.vectorize.predicate.enable", i1 0}
+   !1 = !{!"llvm.loop.vectorize.predicate.enable", i1 1}
+
 '``llvm.loop.vectorize.width``' Metadata
 
 
Index: cfe/trunk/docs/LanguageExtensions.rst
===
--- cfe/trunk/docs/LanguageExtensions.rst
+++ cfe/trunk/docs/LanguageExtensions.rst
@@ -2946,12 +2946,12 @@
 
 The ``#pragma clang loop`` directive is used to specify hints for optimizing the
 subsequent for, while, do-while, or c++11 range-based for loop. The directive
-provides options for vectorization, interleaving, unrolling and
+provides options for vectorization, interleaving, predication, unrolling and
 distribution. Loop hints can be specified before any loop and will be ignored if
 the optimization is not safe to apply.
 
-Vectorization and Interleaving
---
+Vectorization, Interleaving, and Predication
+
 
 A vectorized loop performs multiple iterations of the original loop
 in parallel using vector instructions. The instruction set of the target
@@ -2994,6 +2994,21 @@
 Specifying a width/count of 1 disables the optimization, and is equivalent to
 ``vectorize(disable)`` or ``interleave(disable)``.
 
+Vector predication is enabled by ``vectorize_predicate(enable)``, for example:
+
+.. code-block:: c++
+
+  #pragma clang loop vectorize(enable)
+  #pragma clang loop vectorize_predicate(enable)
+  for(...) {
+...
+  }
+
+This predicates (masks) all instructions in the loop, which allows the scalar
+remainder loop (the tail) to be folded into the main vectorized loop. This
+might be more efficient when vector predication is efficiently supported by the
+target platform.
+
 Loop Unrolling
 --
 
Index: cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td
===
--- cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td
+++ cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td
@@ -1210,7 +1210,7 @@
 def err_pragma_loop_invalid_option : Error<
   "%select{invalid|missing}0 option%select{ %1|}0; expected vectorize, "
   "vectorize_width, interleave, interleave_count, unroll, unroll_count, "
-  "pipeline, pipeline_initiation_interval, or distribute">;
+  "pipeline, pipeline_initiation_interval, vectorize_predicate, or distribute">;
 
 def err_pragma_fp_invalid_option : Error<
   "%select{invalid|missing}0 option%select{ %1|}0; expected contract">;
Index: cfe/trunk/include/clang/Basic/Attr.td
===
--- cfe/trunk/include/clang/Basic/Attr.td
+++ cfe/trunk/include/clang/Basic/Attr.td
@@ -2982,10 +2982,12 @@
   let Args = [EnumArgument<"Option", "OptionType",
   ["vectorize", "vectorize_width", "interleave", "interleave_count",
"unroll", "unroll_count", "unroll_and_jam", "unroll_and_jam_count",
-   "pipeline", "pipeline_initiation_interval", "distribute"],
+   "pipeline", "pipeline_initiation_interval", "distribute",
+   "vectorize_predicate"],
   

[PATCH] D64744: #pragma clang loop vectorize_predicate(enable|disable)

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer added a comment.

Many thanks for reviewing!


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64744/new/

https://reviews.llvm.org/D64744



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D64744: #pragma clang loop vectorize_predicate(enable|disable)

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer added a comment.

Apologies for the early ping! Bu I'm off next weeks, so it would be nice to get 
this in before that if there are no further comments.
Tomorrow, I will upload another diff that builds on top D64916 
, which enables code-generation with tail 
loops folded, thus also demonstrating an end to end test.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64744/new/

https://reviews.llvm.org/D64744



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D64744: #pragma clang loop vectorize_predicate(enable|disable)

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer updated this revision to Diff 211096.
SjoerdMeijer added a comment.

More doc changes added to `AttrDocs.td` and `LangRef.rst`


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64744/new/

https://reviews.llvm.org/D64744

Files:
  clang/docs/LanguageExtensions.rst
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/lib/CodeGen/CGLoopInfo.cpp
  clang/lib/CodeGen/CGLoopInfo.h
  clang/lib/Parse/ParsePragma.cpp
  clang/lib/Sema/SemaStmtAttr.cpp
  clang/test/AST/ast-print-pragmas.cpp
  clang/test/CodeGenCXX/pragma-loop-predicate.cpp
  clang/test/Parser/pragma-loop.cpp
  clang/test/Parser/pragma-unroll-and-jam.cpp
  llvm/docs/LangRef.rst

Index: llvm/docs/LangRef.rst
===
--- llvm/docs/LangRef.rst
+++ llvm/docs/LangRef.rst
@@ -5389,6 +5389,21 @@
!0 = !{!"llvm.loop.vectorize.enable", i1 0}
!1 = !{!"llvm.loop.vectorize.enable", i1 1}
 
+'``llvm.loop.vectorize.predicate.enable``' Metadata
+^^^
+
+This metadata selectively enables or disables creating predicated instructions
+for the loop, which can enable folding of the scalar epilogue loop into the
+main loop. The first operand is the string
+``llvm.loop.vectorize.predicate.enable`` and the second operand is a bit. If
+the bit operand value is 1 vectorization is enabled. A value of 0 disables
+vectorization:
+
+.. code-block:: llvm
+
+   !0 = !{!"llvm.loop.vectorize.predicate.enable", i1 0}
+   !1 = !{!"llvm.loop.vectorize.predicate.enable", i1 1}
+
 '``llvm.loop.vectorize.width``' Metadata
 
 
Index: clang/test/Parser/pragma-unroll-and-jam.cpp
===
--- clang/test/Parser/pragma-unroll-and-jam.cpp
+++ clang/test/Parser/pragma-unroll-and-jam.cpp
@@ -67,7 +67,7 @@
   }
 
 // pragma clang unroll_and_jam is disabled for the moment
-/* expected-error {{invalid option 'unroll_and_jam'; expected vectorize, vectorize_width, interleave, interleave_count, unroll, unroll_count, pipeline, pipeline_initiation_interval, or distribute}} */ #pragma clang loop unroll_and_jam(4)
+/* expected-error {{invalid option 'unroll_and_jam'; expected vectorize, vectorize_width, interleave, interleave_count, unroll, unroll_count, pipeline, pipeline_initiation_interval, vectorize_predicate, or distribute}} */ #pragma clang loop unroll_and_jam(4)
   for (int i = 0; i < Length; i++) {
 for (int j = 0; j < Length; j++) {
   List[i * Length + j] = Value;
Index: clang/test/Parser/pragma-loop.cpp
===
--- clang/test/Parser/pragma-loop.cpp
+++ clang/test/Parser/pragma-loop.cpp
@@ -81,6 +81,7 @@
 
 #pragma clang loop vectorize(enable)
 #pragma clang loop interleave(enable)
+#pragma clang loop vectorize_predicate(enable)
 #pragma clang loop unroll(full)
   while (i + 1 < Length) {
 List[i] = i;
@@ -95,6 +96,7 @@
 
 #pragma clang loop vectorize(disable)
 #pragma clang loop interleave(disable)
+#pragma clang loop vectorize_predicate(disable)
 #pragma clang loop unroll(disable)
   while (i - 1 < Length) {
 List[i] = i;
@@ -111,7 +113,7 @@
   }
 
   int VList[Length];
-#pragma clang loop vectorize(disable) interleave(disable) unroll(disable)
+#pragma clang loop vectorize(disable) interleave(disable) unroll(disable) vectorize_predicate(disable)
   for (int j : VList) {
 VList[j] = List[j];
   }
@@ -130,11 +132,13 @@
 
 /* expected-error {{expected '('}} */ #pragma clang loop vectorize
 /* expected-error {{expected '('}} */ #pragma clang loop interleave
+/* expected-error {{expected '('}} */ #pragma clang loop vectorize_predicate
 /* expected-error {{expected '('}} */ #pragma clang loop unroll
 /* expected-error {{expected '('}} */ #pragma clang loop distribute
 
 /* expected-error {{expected ')'}} */ #pragma clang loop vectorize(enable
 /* expected-error {{expected ')'}} */ #pragma clang loop interleave(enable
+/* expected-error {{expected ')'}} */ #pragma clang loop vectorize_predicate(enable
 /* expected-error {{expected ')'}} */ #pragma clang loop unroll(full
 /* expected-error {{expected ')'}} */ #pragma clang loop distribute(enable
 
@@ -147,7 +151,7 @@
 /* expected-error {{missing argument; expected 'enable', 'full' or 'disable'}} */ #pragma clang loop unroll()
 /* expected-error {{missing argument; expected 'enable' or 'disable'}} */ #pragma clang loop distribute()
 
-/* expected-error {{missing option; expected vectorize, vectorize_width, interleave, interleave_count, unroll, unroll_count, pipeline, pipeline_initiation_interval, or distribute}} */ #pragma clang loop
+/* expected-error {{missing option; expected vectorize, vectorize_width, interleave, interleave_count, unroll, unroll_count, pipeline, pipeline_initiation_interval, vectorize_predicate, or distribute}} */ #pragma clang loop
 /* 

[PATCH] D64744: #pragma clang loop vectorize_predicate(enable|disable)

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer added a comment.

> Is it intentional that this review has no reviewers listed (like, is this a 
> work in progress you don't expect review on yet)?

No, sorry about this, that's not intentional. It started indeed as a 
work-in-progress patch when I wrote to the clang/llvm with an idea about this. 
I wanted to show some code too, but didn't add reviewers at that point. In the 
mean time, this patch has evolved quite a bit, but I never bothered to add 
reviewers, but will do.

> You need to update AttrDocs.td for this as well.

Thanks for the suggestion! Will do.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64744/new/

https://reviews.llvm.org/D64744



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D64744: #pragma clang loop vectorize_predicate(enable|disable)

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer updated this revision to Diff 211042.
SjoerdMeijer added a comment.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

- Moved the codegen test to a separate file
- Added a langref description for this new metadata node.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64744/new/

https://reviews.llvm.org/D64744

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/lib/CodeGen/CGLoopInfo.cpp
  clang/lib/CodeGen/CGLoopInfo.h
  clang/lib/Parse/ParsePragma.cpp
  clang/lib/Sema/SemaStmtAttr.cpp
  clang/test/AST/ast-print-pragmas.cpp
  clang/test/CodeGenCXX/pragma-loop-predicate.cpp
  clang/test/Parser/pragma-loop.cpp
  clang/test/Parser/pragma-unroll-and-jam.cpp
  llvm/docs/LangRef.rst

Index: llvm/docs/LangRef.rst
===
--- llvm/docs/LangRef.rst
+++ llvm/docs/LangRef.rst
@@ -5389,6 +5389,21 @@
!0 = !{!"llvm.loop.vectorize.enable", i1 0}
!1 = !{!"llvm.loop.vectorize.enable", i1 1}
 
+'``llvm.loop.vectorize.predicate.enable``' Metadata
+^^^
+
+This metadata selectively enables or disables creating predicated instructions
+for the loop, which can enable folding of the scalar epilogue loop into the
+main loop. The first operand is the string
+``llvm.loop.vectorize.predicate.enable`` and the second operand is a bit. If
+the bit operand value is 1 vectorization is enabled. A value of 0 disables
+vectorization:
+
+.. code-block:: llvm
+
+   !0 = !{!"llvm.loop.vectorize.predicate.enable", i1 0}
+   !1 = !{!"llvm.loop.vectorize.predicate.enable", i1 1}
+
 '``llvm.loop.vectorize.width``' Metadata
 
 
Index: clang/test/Parser/pragma-unroll-and-jam.cpp
===
--- clang/test/Parser/pragma-unroll-and-jam.cpp
+++ clang/test/Parser/pragma-unroll-and-jam.cpp
@@ -67,7 +67,7 @@
   }
 
 // pragma clang unroll_and_jam is disabled for the moment
-/* expected-error {{invalid option 'unroll_and_jam'; expected vectorize, vectorize_width, interleave, interleave_count, unroll, unroll_count, pipeline, pipeline_initiation_interval, or distribute}} */ #pragma clang loop unroll_and_jam(4)
+/* expected-error {{invalid option 'unroll_and_jam'; expected vectorize, vectorize_width, interleave, interleave_count, unroll, unroll_count, pipeline, pipeline_initiation_interval, vectorize_predicate, or distribute}} */ #pragma clang loop unroll_and_jam(4)
   for (int i = 0; i < Length; i++) {
 for (int j = 0; j < Length; j++) {
   List[i * Length + j] = Value;
Index: clang/test/Parser/pragma-loop.cpp
===
--- clang/test/Parser/pragma-loop.cpp
+++ clang/test/Parser/pragma-loop.cpp
@@ -81,6 +81,7 @@
 
 #pragma clang loop vectorize(enable)
 #pragma clang loop interleave(enable)
+#pragma clang loop vectorize_predicate(enable)
 #pragma clang loop unroll(full)
   while (i + 1 < Length) {
 List[i] = i;
@@ -95,6 +96,7 @@
 
 #pragma clang loop vectorize(disable)
 #pragma clang loop interleave(disable)
+#pragma clang loop vectorize_predicate(disable)
 #pragma clang loop unroll(disable)
   while (i - 1 < Length) {
 List[i] = i;
@@ -111,7 +113,7 @@
   }
 
   int VList[Length];
-#pragma clang loop vectorize(disable) interleave(disable) unroll(disable)
+#pragma clang loop vectorize(disable) interleave(disable) unroll(disable) vectorize_predicate(disable)
   for (int j : VList) {
 VList[j] = List[j];
   }
@@ -130,11 +132,13 @@
 
 /* expected-error {{expected '('}} */ #pragma clang loop vectorize
 /* expected-error {{expected '('}} */ #pragma clang loop interleave
+/* expected-error {{expected '('}} */ #pragma clang loop vectorize_predicate
 /* expected-error {{expected '('}} */ #pragma clang loop unroll
 /* expected-error {{expected '('}} */ #pragma clang loop distribute
 
 /* expected-error {{expected ')'}} */ #pragma clang loop vectorize(enable
 /* expected-error {{expected ')'}} */ #pragma clang loop interleave(enable
+/* expected-error {{expected ')'}} */ #pragma clang loop vectorize_predicate(enable
 /* expected-error {{expected ')'}} */ #pragma clang loop unroll(full
 /* expected-error {{expected ')'}} */ #pragma clang loop distribute(enable
 
@@ -147,7 +151,7 @@
 /* expected-error {{missing argument; expected 'enable', 'full' or 'disable'}} */ #pragma clang loop unroll()
 /* expected-error {{missing argument; expected 'enable' or 'disable'}} */ #pragma clang loop distribute()
 
-/* expected-error {{missing option; expected vectorize, vectorize_width, interleave, interleave_count, unroll, unroll_count, pipeline, pipeline_initiation_interval, or distribute}} */ #pragma clang loop
+/* expected-error {{missing option; expected vectorize, vectorize_width, interleave, interleave_count, unroll, unroll_count, pipeline, pipeline_initiation_interval, vectorize_predicate, or 

[PATCH] D64744: #pragma clang loop vectorize_predicate(enable|disable)

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer updated this revision to Diff 210857.
SjoerdMeijer added a comment.

Removed the separate function that created the loop.llvm.vectorize.predicate 
metadata. This is now just part of function `createLoopVectorizeMetadata`, that 
creates all other vectorize metadata.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64744/new/

https://reviews.llvm.org/D64744

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/lib/CodeGen/CGLoopInfo.cpp
  clang/lib/CodeGen/CGLoopInfo.h
  clang/lib/Parse/ParsePragma.cpp
  clang/lib/Sema/SemaStmtAttr.cpp
  clang/test/AST/ast-print-pragmas.cpp
  clang/test/CodeGenCXX/pragma-loop.cpp
  clang/test/Parser/pragma-loop.cpp
  clang/test/Parser/pragma-unroll-and-jam.cpp

Index: clang/test/Parser/pragma-unroll-and-jam.cpp
===
--- clang/test/Parser/pragma-unroll-and-jam.cpp
+++ clang/test/Parser/pragma-unroll-and-jam.cpp
@@ -67,7 +67,7 @@
   }
 
 // pragma clang unroll_and_jam is disabled for the moment
-/* expected-error {{invalid option 'unroll_and_jam'; expected vectorize, vectorize_width, interleave, interleave_count, unroll, unroll_count, pipeline, pipeline_initiation_interval, or distribute}} */ #pragma clang loop unroll_and_jam(4)
+/* expected-error {{invalid option 'unroll_and_jam'; expected vectorize, vectorize_width, interleave, interleave_count, unroll, unroll_count, pipeline, pipeline_initiation_interval, vectorize_predicate, or distribute}} */ #pragma clang loop unroll_and_jam(4)
   for (int i = 0; i < Length; i++) {
 for (int j = 0; j < Length; j++) {
   List[i * Length + j] = Value;
Index: clang/test/Parser/pragma-loop.cpp
===
--- clang/test/Parser/pragma-loop.cpp
+++ clang/test/Parser/pragma-loop.cpp
@@ -81,6 +81,7 @@
 
 #pragma clang loop vectorize(enable)
 #pragma clang loop interleave(enable)
+#pragma clang loop vectorize_predicate(enable)
 #pragma clang loop unroll(full)
   while (i + 1 < Length) {
 List[i] = i;
@@ -95,6 +96,7 @@
 
 #pragma clang loop vectorize(disable)
 #pragma clang loop interleave(disable)
+#pragma clang loop vectorize_predicate(disable)
 #pragma clang loop unroll(disable)
   while (i - 1 < Length) {
 List[i] = i;
@@ -111,7 +113,7 @@
   }
 
   int VList[Length];
-#pragma clang loop vectorize(disable) interleave(disable) unroll(disable)
+#pragma clang loop vectorize(disable) interleave(disable) unroll(disable) vectorize_predicate(disable)
   for (int j : VList) {
 VList[j] = List[j];
   }
@@ -130,11 +132,13 @@
 
 /* expected-error {{expected '('}} */ #pragma clang loop vectorize
 /* expected-error {{expected '('}} */ #pragma clang loop interleave
+/* expected-error {{expected '('}} */ #pragma clang loop vectorize_predicate
 /* expected-error {{expected '('}} */ #pragma clang loop unroll
 /* expected-error {{expected '('}} */ #pragma clang loop distribute
 
 /* expected-error {{expected ')'}} */ #pragma clang loop vectorize(enable
 /* expected-error {{expected ')'}} */ #pragma clang loop interleave(enable
+/* expected-error {{expected ')'}} */ #pragma clang loop vectorize_predicate(enable
 /* expected-error {{expected ')'}} */ #pragma clang loop unroll(full
 /* expected-error {{expected ')'}} */ #pragma clang loop distribute(enable
 
@@ -147,7 +151,7 @@
 /* expected-error {{missing argument; expected 'enable', 'full' or 'disable'}} */ #pragma clang loop unroll()
 /* expected-error {{missing argument; expected 'enable' or 'disable'}} */ #pragma clang loop distribute()
 
-/* expected-error {{missing option; expected vectorize, vectorize_width, interleave, interleave_count, unroll, unroll_count, pipeline, pipeline_initiation_interval, or distribute}} */ #pragma clang loop
+/* expected-error {{missing option; expected vectorize, vectorize_width, interleave, interleave_count, unroll, unroll_count, pipeline, pipeline_initiation_interval, vectorize_predicate, or distribute}} */ #pragma clang loop
 /* expected-error {{invalid option 'badkeyword'}} */ #pragma clang loop badkeyword
 /* expected-error {{invalid option 'badkeyword'}} */ #pragma clang loop badkeyword(enable)
 /* expected-error {{invalid option 'badkeyword'}} */ #pragma clang loop vectorize(enable) badkeyword(4)
@@ -245,6 +249,8 @@
 /* expected-error {{duplicate directives 'vectorize(enable)' and 'vectorize(disable)'}} */ #pragma clang loop vectorize(disable)
 #pragma clang loop interleave(enable)
 /* expected-error {{duplicate directives 'interleave(enable)' and 'interleave(disable)'}} */ #pragma clang loop interleave(disable)
+#pragma clang loop vectorize_predicate(enable)
+/* expected-error {{duplicate directives 'vectorize_predicate(enable)' and 'vectorize_predicate(disable)'}} */ #pragma clang loop vectorize_predicate(disable)
 #pragma clang loop unroll(full)
 /* expected-error {{duplicate directives 'unroll(full)' and 'unroll(disable)'}} */ #pragma clang loop unroll(disable)

[PATCH] D64744: #pragma clang loop vectorize_predicate(enable|disable)

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer updated this revision to Diff 210774.
SjoerdMeijer retitled this revision from " #pragma clang loop 
predicate(enable|disable)" to " #pragma clang loop 
vectorize_predicate(enable|disable)".
SjoerdMeijer edited the summary of this revision.
SjoerdMeijer added a comment.

Hi Michael, thanks for taking a look again!

Completely agree what you suggested, so I've change the pragma to 
`vectorize_predicate(enable) ` and the metadata to 
`llvm.loop.vectorize.predicate`.

My little plan is as follows:

- Finish D64916 : thanks for reviewing that 
too!
- I will follow up on that. I have a local patch that I need to finish that 
shows how all moving parts work together. I.e., it picks up the metadata, and 
enables the folding.
- Finally, I was thinking to follow up with a doc patch. I don't want to 
advertise this just yet, but want to get everything in, and then add it to the 
docs.

Does that sound like an idea?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64744/new/

https://reviews.llvm.org/D64744

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/lib/CodeGen/CGLoopInfo.cpp
  clang/lib/CodeGen/CGLoopInfo.h
  clang/lib/Parse/ParsePragma.cpp
  clang/lib/Sema/SemaStmtAttr.cpp
  clang/test/AST/ast-print-pragmas.cpp
  clang/test/CodeGenCXX/pragma-loop.cpp
  clang/test/Parser/pragma-loop.cpp
  clang/test/Parser/pragma-unroll-and-jam.cpp

Index: clang/test/Parser/pragma-unroll-and-jam.cpp
===
--- clang/test/Parser/pragma-unroll-and-jam.cpp
+++ clang/test/Parser/pragma-unroll-and-jam.cpp
@@ -67,7 +67,7 @@
   }
 
 // pragma clang unroll_and_jam is disabled for the moment
-/* expected-error {{invalid option 'unroll_and_jam'; expected vectorize, vectorize_width, interleave, interleave_count, unroll, unroll_count, pipeline, pipeline_initiation_interval, or distribute}} */ #pragma clang loop unroll_and_jam(4)
+/* expected-error {{invalid option 'unroll_and_jam'; expected vectorize, vectorize_width, interleave, interleave_count, unroll, unroll_count, pipeline, pipeline_initiation_interval, vectorize_predicate, or distribute}} */ #pragma clang loop unroll_and_jam(4)
   for (int i = 0; i < Length; i++) {
 for (int j = 0; j < Length; j++) {
   List[i * Length + j] = Value;
Index: clang/test/Parser/pragma-loop.cpp
===
--- clang/test/Parser/pragma-loop.cpp
+++ clang/test/Parser/pragma-loop.cpp
@@ -81,6 +81,7 @@
 
 #pragma clang loop vectorize(enable)
 #pragma clang loop interleave(enable)
+#pragma clang loop vectorize_predicate(enable)
 #pragma clang loop unroll(full)
   while (i + 1 < Length) {
 List[i] = i;
@@ -95,6 +96,7 @@
 
 #pragma clang loop vectorize(disable)
 #pragma clang loop interleave(disable)
+#pragma clang loop vectorize_predicate(disable)
 #pragma clang loop unroll(disable)
   while (i - 1 < Length) {
 List[i] = i;
@@ -111,7 +113,7 @@
   }
 
   int VList[Length];
-#pragma clang loop vectorize(disable) interleave(disable) unroll(disable)
+#pragma clang loop vectorize(disable) interleave(disable) unroll(disable) vectorize_predicate(disable)
   for (int j : VList) {
 VList[j] = List[j];
   }
@@ -130,11 +132,13 @@
 
 /* expected-error {{expected '('}} */ #pragma clang loop vectorize
 /* expected-error {{expected '('}} */ #pragma clang loop interleave
+/* expected-error {{expected '('}} */ #pragma clang loop vectorize_predicate
 /* expected-error {{expected '('}} */ #pragma clang loop unroll
 /* expected-error {{expected '('}} */ #pragma clang loop distribute
 
 /* expected-error {{expected ')'}} */ #pragma clang loop vectorize(enable
 /* expected-error {{expected ')'}} */ #pragma clang loop interleave(enable
+/* expected-error {{expected ')'}} */ #pragma clang loop vectorize_predicate(enable
 /* expected-error {{expected ')'}} */ #pragma clang loop unroll(full
 /* expected-error {{expected ')'}} */ #pragma clang loop distribute(enable
 
@@ -147,7 +151,7 @@
 /* expected-error {{missing argument; expected 'enable', 'full' or 'disable'}} */ #pragma clang loop unroll()
 /* expected-error {{missing argument; expected 'enable' or 'disable'}} */ #pragma clang loop distribute()
 
-/* expected-error {{missing option; expected vectorize, vectorize_width, interleave, interleave_count, unroll, unroll_count, pipeline, pipeline_initiation_interval, or distribute}} */ #pragma clang loop
+/* expected-error {{missing option; expected vectorize, vectorize_width, interleave, interleave_count, unroll, unroll_count, pipeline, pipeline_initiation_interval, vectorize_predicate, or distribute}} */ #pragma clang loop
 /* expected-error {{invalid option 'badkeyword'}} */ #pragma clang loop badkeyword
 /* expected-error {{invalid option 'badkeyword'}} */ #pragma clang loop badkeyword(enable)
 /* expected-error {{invalid option 'badkeyword'}} */ #pragma clang loop vectorize(enable) 

[PATCH] D64744: #pragma clang loop predicate(enable|disable)

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer added a comment.

So I went for:

  predicate(enable)

as I think that is most general, best capturing it, but I am of course 
completely open to bikeshedding names :-)


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64744/new/

https://reviews.llvm.org/D64744



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D64744: #pragma clang loop predicate(enable|disable)

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer updated this revision to Diff 210314.
SjoerdMeijer retitled this revision from "Loop #pragma tail_predicate" to " 
#pragma clang loop predicate(enable|disable)".
SjoerdMeijer edited the summary of this revision.
Herald added a subscriber: zzheng.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64744/new/

https://reviews.llvm.org/D64744

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/lib/CodeGen/CGLoopInfo.cpp
  clang/lib/CodeGen/CGLoopInfo.h
  clang/lib/Parse/ParsePragma.cpp
  clang/lib/Sema/SemaStmtAttr.cpp
  clang/test/AST/ast-print-pragmas.cpp
  clang/test/CodeGenCXX/pragma-loop.cpp
  clang/test/Parser/pragma-loop.cpp
  clang/test/Parser/pragma-unroll-and-jam.cpp

Index: clang/test/Parser/pragma-unroll-and-jam.cpp
===
--- clang/test/Parser/pragma-unroll-and-jam.cpp
+++ clang/test/Parser/pragma-unroll-and-jam.cpp
@@ -67,7 +67,7 @@
   }
 
 // pragma clang unroll_and_jam is disabled for the moment
-/* expected-error {{invalid option 'unroll_and_jam'; expected vectorize, vectorize_width, interleave, interleave_count, unroll, unroll_count, pipeline, pipeline_initiation_interval, or distribute}} */ #pragma clang loop unroll_and_jam(4)
+/* expected-error {{invalid option 'unroll_and_jam'; expected vectorize, vectorize_width, interleave, interleave_count, unroll, unroll_count, pipeline, pipeline_initiation_interval, predicate, or distribute}} */ #pragma clang loop unroll_and_jam(4)
   for (int i = 0; i < Length; i++) {
 for (int j = 0; j < Length; j++) {
   List[i * Length + j] = Value;
Index: clang/test/Parser/pragma-loop.cpp
===
--- clang/test/Parser/pragma-loop.cpp
+++ clang/test/Parser/pragma-loop.cpp
@@ -81,6 +81,7 @@
 
 #pragma clang loop vectorize(enable)
 #pragma clang loop interleave(enable)
+#pragma clang loop predicate(enable)
 #pragma clang loop unroll(full)
   while (i + 1 < Length) {
 List[i] = i;
@@ -95,6 +96,7 @@
 
 #pragma clang loop vectorize(disable)
 #pragma clang loop interleave(disable)
+#pragma clang loop predicate(disable)
 #pragma clang loop unroll(disable)
   while (i - 1 < Length) {
 List[i] = i;
@@ -111,7 +113,7 @@
   }
 
   int VList[Length];
-#pragma clang loop vectorize(disable) interleave(disable) unroll(disable)
+#pragma clang loop vectorize(disable) interleave(disable) unroll(disable) predicate(disable)
   for (int j : VList) {
 VList[j] = List[j];
   }
@@ -130,11 +132,13 @@
 
 /* expected-error {{expected '('}} */ #pragma clang loop vectorize
 /* expected-error {{expected '('}} */ #pragma clang loop interleave
+/* expected-error {{expected '('}} */ #pragma clang loop predicate
 /* expected-error {{expected '('}} */ #pragma clang loop unroll
 /* expected-error {{expected '('}} */ #pragma clang loop distribute
 
 /* expected-error {{expected ')'}} */ #pragma clang loop vectorize(enable
 /* expected-error {{expected ')'}} */ #pragma clang loop interleave(enable
+/* expected-error {{expected ')'}} */ #pragma clang loop predicate(enable
 /* expected-error {{expected ')'}} */ #pragma clang loop unroll(full
 /* expected-error {{expected ')'}} */ #pragma clang loop distribute(enable
 
@@ -147,7 +151,7 @@
 /* expected-error {{missing argument; expected 'enable', 'full' or 'disable'}} */ #pragma clang loop unroll()
 /* expected-error {{missing argument; expected 'enable' or 'disable'}} */ #pragma clang loop distribute()
 
-/* expected-error {{missing option; expected vectorize, vectorize_width, interleave, interleave_count, unroll, unroll_count, pipeline, pipeline_initiation_interval, or distribute}} */ #pragma clang loop
+/* expected-error {{missing option; expected vectorize, vectorize_width, interleave, interleave_count, unroll, unroll_count, pipeline, pipeline_initiation_interval, predicate, or distribute}} */ #pragma clang loop
 /* expected-error {{invalid option 'badkeyword'}} */ #pragma clang loop badkeyword
 /* expected-error {{invalid option 'badkeyword'}} */ #pragma clang loop badkeyword(enable)
 /* expected-error {{invalid option 'badkeyword'}} */ #pragma clang loop vectorize(enable) badkeyword(4)
@@ -245,6 +249,8 @@
 /* expected-error {{duplicate directives 'vectorize(enable)' and 'vectorize(disable)'}} */ #pragma clang loop vectorize(disable)
 #pragma clang loop interleave(enable)
 /* expected-error {{duplicate directives 'interleave(enable)' and 'interleave(disable)'}} */ #pragma clang loop interleave(disable)
+#pragma clang loop predicate(enable)
+/* expected-error {{duplicate directives 'predicate(enable)' and 'predicate(disable)'}} */ #pragma clang loop predicate(disable)
 #pragma clang loop unroll(full)
 /* expected-error {{duplicate directives 'unroll(full)' and 'unroll(disable)'}} */ #pragma clang loop unroll(disable)
 #pragma clang loop distribute(enable)
@@ -281,3 +287,7 @@
 
 #pragma clang loop interleave(enable)
 /* expected-error {{expected 

[PATCH] D64744: Loop #pragma tail_predicate

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer added a comment.

Thanks for taking a look and your suggestions!

I noticed your comment here after I replied to the list. As I wrote there, and 
long story short, I thought I could kill 2 birds with 1 stone, but that doesn't 
seem to be the case. I agree that for the vectorizer an option like this is 
much better:

  #pragma clang loop vectorize(enable) vectorize_remainder(enable)

I will implement that here because that looks a useful addition that people 
like, and I think this will also work for me initially.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64744/new/

https://reviews.llvm.org/D64744



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D64744: Loop #pragma tail_predicate

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer created this revision.

https://reviews.llvm.org/D64744

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Parse/Parser.h
  clang/lib/CodeGen/CGLoopInfo.cpp
  clang/lib/CodeGen/CGLoopInfo.h
  clang/lib/Parse/ParsePragma.cpp
  clang/lib/Sema/SemaStmtAttr.cpp
  clang/test/CodeGenCXX/pragma-tail-predicate.cpp
  clang/test/Parser/pragma-tail-predicate.cpp

Index: clang/test/Parser/pragma-tail-predicate.cpp
===
--- /dev/null
+++ clang/test/Parser/pragma-tail-predicate.cpp
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 -std=c++11 -verify %s
+
+// Note that this puts the expected lines before the directives to work around
+// limitations in the -verify mode.
+
+int foo(int *a, int N) {
+  int sum = 0;
+
+  #pragma tail_predicate
+  for (int i = 0; i < N; i++)
+sum += a[i];
+
+
+  #pragma tail_predicate
+  /* expected-error {{expected a for, while, or do-while loop to follow '#pragma tail_predicate'}} */ int b;
+
+  return sum;
+}
Index: clang/test/CodeGenCXX/pragma-tail-predicate.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/pragma-tail-predicate.cpp
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -triple arm-none-eabi -std=c++11 -emit-llvm -o - %s | FileCheck %s
+
+// CHECK:  !{!"llvm.loop.tailpredicate"}
+
+int foo(int *a, int N) {
+  int sum = 0;
+
+  #pragma tail_predicate
+  for (int i = 0; i < N; i++)
+sum += a[i];
+
+  return sum;
+}
Index: clang/lib/Sema/SemaStmtAttr.cpp
===
--- clang/lib/Sema/SemaStmtAttr.cpp
+++ clang/lib/Sema/SemaStmtAttr.cpp
@@ -85,6 +85,7 @@
   StringRef PragmaName =
   llvm::StringSwitch(PragmaNameLoc->Ident->getName())
   .Cases("unroll", "nounroll", "unroll_and_jam", "nounroll_and_jam",
+ "tail_predicate",
  PragmaNameLoc->Ident->getName())
   .Default("clang loop");
 
@@ -118,6 +119,8 @@
   SetHints(LoopHintAttr::Unroll, LoopHintAttr::Enable);
   } else if (PragmaName == "nounroll_and_jam") {
 SetHints(LoopHintAttr::UnrollAndJam, LoopHintAttr::Disable);
+  } else if (PragmaName == "tail_predicate") {
+SetHints(LoopHintAttr::TailPredicate, LoopHintAttr::Enable);
   } else if (PragmaName == "unroll_and_jam") {
 // #pragma unroll_and_jam N
 if (ValueExpr)
@@ -189,7 +192,8 @@
 const LoopHintAttr *StateAttr;
 const LoopHintAttr *NumericAttr;
   } HintAttrs[] = {{nullptr, nullptr}, {nullptr, nullptr}, {nullptr, nullptr},
-   {nullptr, nullptr}, {nullptr, nullptr}, {nullptr, nullptr}};
+   {nullptr, nullptr}, {nullptr, nullptr}, {nullptr, nullptr},
+   {nullptr, nullptr}};
 
   for (const auto *I : Attrs) {
 const LoopHintAttr *LH = dyn_cast(I);
@@ -205,7 +209,8 @@
   Unroll,
   UnrollAndJam,
   Distribute,
-  Pipeline
+  Pipeline,
+  TailPredicate
 } Category;
 switch (Option) {
 case LoopHintAttr::Vectorize:
@@ -232,6 +237,9 @@
 case LoopHintAttr::PipelineInitiationInterval:
   Category = Pipeline;
   break;
+case LoopHintAttr::TailPredicate:
+  Category = TailPredicate;
+  break;
 };
 
 assert(Category < sizeof(HintAttrs) / sizeof(HintAttrs[0]));
@@ -240,6 +248,7 @@
 if (Option == LoopHintAttr::Vectorize ||
 Option == LoopHintAttr::Interleave || Option == LoopHintAttr::Unroll ||
 Option == LoopHintAttr::UnrollAndJam ||
+Option == LoopHintAttr::TailPredicate ||
 Option == LoopHintAttr::PipelineDisabled ||
 Option == LoopHintAttr::Distribute) {
   // Enable|Disable|AssumeSafety hint.  For example, vectorize(enable).
Index: clang/lib/Parse/ParsePragma.cpp
===
--- clang/lib/Parse/ParsePragma.cpp
+++ clang/lib/Parse/ParsePragma.cpp
@@ -372,6 +372,10 @@
   llvm::make_unique("unroll_and_jam");
   PP.AddPragmaHandler(UnrollAndJamHintHandler.get());
 
+  TailPredicateHintHandler =
+  llvm::make_unique("tail_predicate");
+  PP.AddPragmaHandler(TailPredicateHintHandler.get());
+
   NoUnrollAndJamHintHandler =
   llvm::make_unique("nounroll_and_jam");
   PP.AddPragmaHandler(NoUnrollAndJamHintHandler.get());
@@ -479,6 +483,9 @@
   PP.RemovePragmaHandler(UnrollAndJamHintHandler.get());
   UnrollAndJamHintHandler.reset();
 
+  PP.RemovePragmaHandler(TailPredicateHintHandler.get());
+  TailPredicateHintHandler.reset();
+
   PP.RemovePragmaHandler(NoUnrollAndJamHintHandler.get());
   NoUnrollAndJamHintHandler.reset();
 
@@ -1006,17 +1013,13 @@
 } // end anonymous namespace
 
 static std::string PragmaLoopHintString(Token PragmaName, Token Option) {
-  std::string PragmaString;
-  if (PragmaName.getIdentifierInfo()->getName() == "loop") {
-PragmaString = "clang loop ";
-PragmaString += Option.getIdentifierInfo()->getName();
-  } else if 

[PATCH] D64564: Loop pragma parsing. NFC.

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer added a comment.

oh no, the diff got messed up here. I.e. Phab shows 2 modified files, because 
the first patch set was created in a monorepo, and the 2nd patch set in a 
another and separate clang repo.

Just for clarity, file `lib/Parse/ParsePragma.cpp` is the latest with the 
feedback addressed.
Tomorrow I will see if I can fix this in Phab...


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64564/new/

https://reviews.llvm.org/D64564



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D64564: Loop pragma parsing. NFC.

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer updated this revision to Diff 209299.
SjoerdMeijer marked an inline comment as not done.
SjoerdMeijer added a comment.

thanks for taking a look and the suggestions!


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64564/new/

https://reviews.llvm.org/D64564

Files:
  lib/Parse/ParsePragma.cpp


Index: lib/Parse/ParsePragma.cpp
===
--- lib/Parse/ParsePragma.cpp
+++ lib/Parse/ParsePragma.cpp
@@ -1006,18 +1006,14 @@
 } // end anonymous namespace
 
 static std::string PragmaLoopHintString(Token PragmaName, Token Option) {
-  std::string PragmaString;
-  if (PragmaName.getIdentifierInfo()->getName() == "loop") {
-PragmaString = "clang loop ";
-PragmaString += Option.getIdentifierInfo()->getName();
-  } else if (PragmaName.getIdentifierInfo()->getName() == "unroll_and_jam") {
-PragmaString = "unroll_and_jam";
-  } else {
-assert(PragmaName.getIdentifierInfo()->getName() == "unroll" &&
-   "Unexpected pragma name");
-PragmaString = "unroll";
-  }
-  return PragmaString;
+  StringRef Str = PragmaName.getIdentifierInfo()->getName();
+  Str = llvm::StringSwitch(Str)
+   .Case("loop", "clang loop " + Str.str())
+   .Case("unroll_and_jam", Str)
+   .Case("unroll", Str)
+   .Default("");
+  assert(Str.size() && "Unexpected pragma name");
+  return std::string(Str);
 }
 
 bool Parser::HandlePragmaLoopHint(LoopHint ) {
@@ -1041,12 +1037,12 @@
 
   // Return a valid hint if pragma unroll or nounroll were specified
   // without an argument.
-  bool PragmaUnroll = PragmaNameInfo->getName() == "unroll";
-  bool PragmaNoUnroll = PragmaNameInfo->getName() == "nounroll";
-  bool PragmaUnrollAndJam = PragmaNameInfo->getName() == "unroll_and_jam";
-  bool PragmaNoUnrollAndJam = PragmaNameInfo->getName() == "nounroll_and_jam";
-  if (Toks.empty() && (PragmaUnroll || PragmaNoUnroll || PragmaUnrollAndJam ||
-   PragmaNoUnrollAndJam)) {
+  auto IsLoopHint = llvm::StringSwitch(PragmaNameInfo->getName())
+.Cases("unroll", "nounroll", "unroll_and_jam",
+   "nounroll_and_jam", true)
+.Default(false);
+
+  if (Toks.empty() && IsLoopHint) {
 ConsumeAnnotationToken();
 Hint.Range = Info->PragmaName.getLocation();
 return true;


Index: lib/Parse/ParsePragma.cpp
===
--- lib/Parse/ParsePragma.cpp
+++ lib/Parse/ParsePragma.cpp
@@ -1006,18 +1006,14 @@
 } // end anonymous namespace
 
 static std::string PragmaLoopHintString(Token PragmaName, Token Option) {
-  std::string PragmaString;
-  if (PragmaName.getIdentifierInfo()->getName() == "loop") {
-PragmaString = "clang loop ";
-PragmaString += Option.getIdentifierInfo()->getName();
-  } else if (PragmaName.getIdentifierInfo()->getName() == "unroll_and_jam") {
-PragmaString = "unroll_and_jam";
-  } else {
-assert(PragmaName.getIdentifierInfo()->getName() == "unroll" &&
-   "Unexpected pragma name");
-PragmaString = "unroll";
-  }
-  return PragmaString;
+  StringRef Str = PragmaName.getIdentifierInfo()->getName();
+  Str = llvm::StringSwitch(Str)
+   .Case("loop", "clang loop " + Str.str())
+   .Case("unroll_and_jam", Str)
+   .Case("unroll", Str)
+   .Default("");
+  assert(Str.size() && "Unexpected pragma name");
+  return std::string(Str);
 }
 
 bool Parser::HandlePragmaLoopHint(LoopHint ) {
@@ -1041,12 +1037,12 @@
 
   // Return a valid hint if pragma unroll or nounroll were specified
   // without an argument.
-  bool PragmaUnroll = PragmaNameInfo->getName() == "unroll";
-  bool PragmaNoUnroll = PragmaNameInfo->getName() == "nounroll";
-  bool PragmaUnrollAndJam = PragmaNameInfo->getName() == "unroll_and_jam";
-  bool PragmaNoUnrollAndJam = PragmaNameInfo->getName() == "nounroll_and_jam";
-  if (Toks.empty() && (PragmaUnroll || PragmaNoUnroll || PragmaUnrollAndJam ||
-   PragmaNoUnrollAndJam)) {
+  auto IsLoopHint = llvm::StringSwitch(PragmaNameInfo->getName())
+.Cases("unroll", "nounroll", "unroll_and_jam",
+   "nounroll_and_jam", true)
+.Default(false);
+
+  if (Toks.empty() && IsLoopHint) {
 ConsumeAnnotationToken();
 Hint.Range = Info->PragmaName.getLocation();
 return true;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D64564: Loop pragma parsing. NFC.

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer marked an inline comment as done.
SjoerdMeijer added inline comments.



Comment at: clang/lib/Parse/ParsePragma.cpp:1015
+   .Default("");
+  assert(Str.size() && "Unexpected pragma name");
+  return Str;

I guess this does behave slightly different, i.e. the assert in a release build.

Before, in a release build, "unroll" was returned for an unexpected pragma, but 
now we return the empty string "".


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64564/new/

https://reviews.llvm.org/D64564



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D64564: Loop pragma parsing. NFC.

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer created this revision.
SjoerdMeijer added reviewers: samparker, dmgreen, Meinersbur.

Yesterday I was a bit distracted by loop pragma parsing (D64471 
), but still am a bit today too (this patch).


https://reviews.llvm.org/D64564

Files:
  clang/lib/Parse/ParsePragma.cpp


Index: clang/lib/Parse/ParsePragma.cpp
===
--- clang/lib/Parse/ParsePragma.cpp
+++ clang/lib/Parse/ParsePragma.cpp
@@ -1006,18 +1006,14 @@
 } // end anonymous namespace
 
 static std::string PragmaLoopHintString(Token PragmaName, Token Option) {
-  std::string PragmaString;
-  if (PragmaName.getIdentifierInfo()->getName() == "loop") {
-PragmaString = "clang loop ";
-PragmaString += Option.getIdentifierInfo()->getName();
-  } else if (PragmaName.getIdentifierInfo()->getName() == "unroll_and_jam") {
-PragmaString = "unroll_and_jam";
-  } else {
-assert(PragmaName.getIdentifierInfo()->getName() == "unroll" &&
-   "Unexpected pragma name");
-PragmaString = "unroll";
-  }
-  return PragmaString;
+  std::string Str = PragmaName.getIdentifierInfo()->getName();
+  Str = llvm::StringSwitch(Str)
+   .Case("loop", std::string("clang loop ") + Str)
+   .Case("unroll_and_jam", Str)
+   .Case("unroll", Str)
+   .Default("");
+  assert(Str.size() && "Unexpected pragma name");
+  return Str;
 }
 
 bool Parser::HandlePragmaLoopHint(LoopHint ) {
@@ -1041,12 +1037,12 @@
 
   // Return a valid hint if pragma unroll or nounroll were specified
   // without an argument.
-  bool PragmaUnroll = PragmaNameInfo->getName() == "unroll";
-  bool PragmaNoUnroll = PragmaNameInfo->getName() == "nounroll";
-  bool PragmaUnrollAndJam = PragmaNameInfo->getName() == "unroll_and_jam";
-  bool PragmaNoUnrollAndJam = PragmaNameInfo->getName() == "nounroll_and_jam";
-  if (Toks.empty() && (PragmaUnroll || PragmaNoUnroll || PragmaUnrollAndJam ||
-   PragmaNoUnrollAndJam)) {
+  const bool LoopHint = llvm::StringSwitch(PragmaNameInfo->getName())
+.Cases("unroll", "nounroll", "unroll_and_jam",
+   "nounroll_and_jam", true)
+.Default(false);
+
+  if (Toks.empty() && LoopHint) {
 ConsumeAnnotationToken();
 Hint.Range = Info->PragmaName.getLocation();
 return true;


Index: clang/lib/Parse/ParsePragma.cpp
===
--- clang/lib/Parse/ParsePragma.cpp
+++ clang/lib/Parse/ParsePragma.cpp
@@ -1006,18 +1006,14 @@
 } // end anonymous namespace
 
 static std::string PragmaLoopHintString(Token PragmaName, Token Option) {
-  std::string PragmaString;
-  if (PragmaName.getIdentifierInfo()->getName() == "loop") {
-PragmaString = "clang loop ";
-PragmaString += Option.getIdentifierInfo()->getName();
-  } else if (PragmaName.getIdentifierInfo()->getName() == "unroll_and_jam") {
-PragmaString = "unroll_and_jam";
-  } else {
-assert(PragmaName.getIdentifierInfo()->getName() == "unroll" &&
-   "Unexpected pragma name");
-PragmaString = "unroll";
-  }
-  return PragmaString;
+  std::string Str = PragmaName.getIdentifierInfo()->getName();
+  Str = llvm::StringSwitch(Str)
+   .Case("loop", std::string("clang loop ") + Str)
+   .Case("unroll_and_jam", Str)
+   .Case("unroll", Str)
+   .Default("");
+  assert(Str.size() && "Unexpected pragma name");
+  return Str;
 }
 
 bool Parser::HandlePragmaLoopHint(LoopHint ) {
@@ -1041,12 +1037,12 @@
 
   // Return a valid hint if pragma unroll or nounroll were specified
   // without an argument.
-  bool PragmaUnroll = PragmaNameInfo->getName() == "unroll";
-  bool PragmaNoUnroll = PragmaNameInfo->getName() == "nounroll";
-  bool PragmaUnrollAndJam = PragmaNameInfo->getName() == "unroll_and_jam";
-  bool PragmaNoUnrollAndJam = PragmaNameInfo->getName() == "nounroll_and_jam";
-  if (Toks.empty() && (PragmaUnroll || PragmaNoUnroll || PragmaUnrollAndJam ||
-   PragmaNoUnrollAndJam)) {
+  const bool LoopHint = llvm::StringSwitch(PragmaNameInfo->getName())
+.Cases("unroll", "nounroll", "unroll_and_jam",
+   "nounroll_and_jam", true)
+.Default(false);
+
+  if (Toks.empty() && LoopHint) {
 ConsumeAnnotationToken();
 Hint.Range = Info->PragmaName.getLocation();
 return true;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D64495: [AArch64] Implement __jcvt intrinsic from Armv8.3-A

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer accepted this revision.
SjoerdMeijer added a comment.
This revision is now accepted and ready to land.

Looks like a good change to me, some nits inlined.

It shouldn't be difficult to request an account and commit it yourself, which 
might be useful if you maybe intend to submit more patches. 
But I can of course easily commit this on your behalf, just let me know. If you 
want me to commit this, it is easiest if you do upload a new diff with the nits 
fixed.




Comment at: clang/test/CodeGen/arm_acle.c:5
 // RUN: %clang_cc1 -ffreestanding -triple aarch64-eabi -target-cpu cortex-a57 
-target-feature +neon -target-feature +crc -target-feature +crypto -O2 
-fexperimental-new-pass-manager -S -emit-llvm -o - %s | FileCheck %s 
-check-prefix=ARM -check-prefix=AArch64 -check-prefix=ARM-NEWPM 
-check-prefix=AArch64-NEWPM
+// RUN: %clang_cc1 -ffreestanding -triple aarch64-eabi -target-cpu cortex-a57 
-target-feature +v8.3a -O2 -fexperimental-new-pass-manager -S -emit-llvm -o - 
%s | FileCheck %s -check-prefix=AArch64-v8_3 -check-prefix=CHECK-LABEL
+// RUN: %clang_cc1 -ffreestanding -triple aarch64-eabi -target-cpu cortex-a57 
-target-feature +v8.4a -O2 -fexperimental-new-pass-manager -S -emit-llvm -o - 
%s | FileCheck %s -check-prefix=AArch64-v8_3 -check-prefix=CHECK-LABEL

Some nits that you can fix before committing; no need for another review I 
think.

You can remove: 

  -check-prefix=CHECK-LABEL

in all these 3 lines below.



Comment at: clang/test/CodeGen/arm_acle.c:830
+
+// CHECK-LABEL: @test_jcvt(
+// AArch64-v8_3: call i32 @llvm.aarch64.fjcvtzs

and change 

  // CHECK-LABEL: @test_jcvt(

into 

   // AArch64-v8_3-LABEL: @test_jcvt(


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64495/new/

https://reviews.llvm.org/D64495



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D64495: [AArch64] Implement __jcvt intrinsic from Armv8.3-A

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer added inline comments.



Comment at: llvm/lib/Target/AArch64/AArch64InstrInfo.td:720
 def FJCVTZS  : BaseFPToIntegerUnscaled<0b01, 0b11, 0b110, FPR64, GPR32,
-  "fjcvtzs", []> {
+  "fjcvtzs", [(set GPR32:$Rd, 
(int_aarch64_fjcvtzs FPR64:$Rn))]> {
   let Inst{31} = 0;

and a last nit: this needs some reformatting (exceeding the max column width)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64495/new/

https://reviews.llvm.org/D64495



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D64495: [AArch64] Implement __jcvt intrinsic from Armv8.3-A

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer added inline comments.



Comment at: clang/lib/Basic/Targets/AArch64.cpp:237
+break;
+  case llvm::AArch64::ArchKind::ARMV8_4A:
+getTargetDefinesARMV84A(Opts, Builder);

SjoerdMeijer wrote:
> It is a good change, but I think you should either add tests for these cases, 
> or remove this (temporarily) because it is not adding much at the moment.
Ah, sorry: "-march=armv8.4-a also enables this"
probably good to add some tests for v8.4 and v8.5 too then.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64495/new/

https://reviews.llvm.org/D64495



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D64495: [AArch64] Implement __jcvt intrinsic from Armv8.3-A

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer added inline comments.



Comment at: clang/lib/Basic/Targets/AArch64.cpp:237
+break;
+  case llvm::AArch64::ArchKind::ARMV8_4A:
+getTargetDefinesARMV84A(Opts, Builder);

It is a good change, but I think you should either add tests for these cases, 
or remove this (temporarily) because it is not adding much at the moment.



Comment at: clang/test/CodeGen/arm_acle.c:829
+
+// AArch64-v8_3: call i32 @llvm.aarch64.fjcvtzs
+#ifdef __ARM_64BIT_STATE

Same comment, better is to do a CHECK-LABEL first



Comment at: clang/test/CodeGen/builtins-arm64.c:61
 
+int32_t jcvt(double v) {
+  //CHECK: call i32 @llvm.aarch64.fjcvtzs

Although this file doesn't seem to do this, better is check the function name 
first, e.g.:

  // CHECK-LABEL: @jcvt(

followed by what you want to check:
  
  //CHECK: call i32 @llvm.aarch64.fjcvtzs


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64495/new/

https://reviews.llvm.org/D64495



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D64471: Loop pragma parsing. NFC.

[Sjoerd Meijer via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL365629: Loop pragma parsing. NFC. (authored by SjoerdMeijer, 
committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D64471?vs=20=208939#toc

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64471/new/

https://reviews.llvm.org/D64471

Files:
  cfe/trunk/lib/Sema/SemaStmtAttr.cpp


Index: cfe/trunk/lib/Sema/SemaStmtAttr.cpp
===
--- cfe/trunk/lib/Sema/SemaStmtAttr.cpp
+++ cfe/trunk/lib/Sema/SemaStmtAttr.cpp
@@ -82,22 +82,17 @@
   IdentifierLoc *StateLoc = A.getArgAsIdent(2);
   Expr *ValueExpr = A.getArgAsExpr(3);
 
-  bool PragmaUnroll = PragmaNameLoc->Ident->getName() == "unroll";
-  bool PragmaNoUnroll = PragmaNameLoc->Ident->getName() == "nounroll";
-  bool PragmaUnrollAndJam = PragmaNameLoc->Ident->getName() == 
"unroll_and_jam";
-  bool PragmaNoUnrollAndJam =
-  PragmaNameLoc->Ident->getName() == "nounroll_and_jam";
+  StringRef PragmaName =
+  llvm::StringSwitch(PragmaNameLoc->Ident->getName())
+  .Cases("unroll", "nounroll", "unroll_and_jam", "nounroll_and_jam",
+ PragmaNameLoc->Ident->getName())
+  .Default("clang loop");
+
   if (St->getStmtClass() != Stmt::DoStmtClass &&
   St->getStmtClass() != Stmt::ForStmtClass &&
   St->getStmtClass() != Stmt::CXXForRangeStmtClass &&
   St->getStmtClass() != Stmt::WhileStmtClass) {
-const char *Pragma =
-llvm::StringSwitch(PragmaNameLoc->Ident->getName())
-.Case("unroll", "#pragma unroll")
-.Case("nounroll", "#pragma nounroll")
-.Case("unroll_and_jam", "#pragma unroll_and_jam")
-.Case("nounroll_and_jam", "#pragma nounroll_and_jam")
-.Default("#pragma clang loop");
+std::string Pragma = "#pragma " + std::string(PragmaName);
 S.Diag(St->getBeginLoc(), diag::err_pragma_loop_precedes_nonloop) << 
Pragma;
 return nullptr;
   }
@@ -106,34 +101,29 @@
   LoopHintAttr::Spelling(A.getAttributeSpellingListIndex());
   LoopHintAttr::OptionType Option;
   LoopHintAttr::LoopHintState State;
-  if (PragmaNoUnroll) {
-// #pragma nounroll
-Option = LoopHintAttr::Unroll;
-State = LoopHintAttr::Disable;
-  } else if (PragmaUnroll) {
-if (ValueExpr) {
-  // #pragma unroll N
-  Option = LoopHintAttr::UnrollCount;
-  State = LoopHintAttr::Numeric;
-} else {
-  // #pragma unroll
-  Option = LoopHintAttr::Unroll;
-  State = LoopHintAttr::Enable;
-}
-  } else if (PragmaNoUnrollAndJam) {
-// #pragma nounroll_and_jam
-Option = LoopHintAttr::UnrollAndJam;
-State = LoopHintAttr::Disable;
-  } else if (PragmaUnrollAndJam) {
-if (ValueExpr) {
-  // #pragma unroll_and_jam N
-  Option = LoopHintAttr::UnrollAndJamCount;
-  State = LoopHintAttr::Numeric;
-} else {
-  // #pragma unroll_and_jam
-  Option = LoopHintAttr::UnrollAndJam;
-  State = LoopHintAttr::Enable;
-}
+
+  auto SetHints = [, ](LoopHintAttr::OptionType O,
+LoopHintAttr::LoopHintState S) {
+Option = O;
+State = S;
+  };
+
+  if (PragmaName == "nounroll") {
+SetHints(LoopHintAttr::Unroll, LoopHintAttr::Disable);
+  } else if (PragmaName == "unroll") {
+// #pragma unroll N
+if (ValueExpr)
+  SetHints(LoopHintAttr::UnrollCount, LoopHintAttr::Numeric);
+else
+  SetHints(LoopHintAttr::Unroll, LoopHintAttr::Enable);
+  } else if (PragmaName == "nounroll_and_jam") {
+SetHints(LoopHintAttr::UnrollAndJam, LoopHintAttr::Disable);
+  } else if (PragmaName == "unroll_and_jam") {
+// #pragma unroll_and_jam N
+if (ValueExpr)
+  SetHints(LoopHintAttr::UnrollAndJamCount, LoopHintAttr::Numeric);
+else
+  SetHints(LoopHintAttr::UnrollAndJam, LoopHintAttr::Enable);
   } else {
 // #pragma clang loop ...
 assert(OptionLoc && OptionLoc->Ident &&


Index: cfe/trunk/lib/Sema/SemaStmtAttr.cpp
===
--- cfe/trunk/lib/Sema/SemaStmtAttr.cpp
+++ cfe/trunk/lib/Sema/SemaStmtAttr.cpp
@@ -82,22 +82,17 @@
   IdentifierLoc *StateLoc = A.getArgAsIdent(2);
   Expr *ValueExpr = A.getArgAsExpr(3);
 
-  bool PragmaUnroll = PragmaNameLoc->Ident->getName() == "unroll";
-  bool PragmaNoUnroll = PragmaNameLoc->Ident->getName() == "nounroll";
-  bool PragmaUnrollAndJam = PragmaNameLoc->Ident->getName() == "unroll_and_jam";
-  bool PragmaNoUnrollAndJam =
-  PragmaNameLoc->Ident->getName() == "nounroll_and_jam";
+  StringRef PragmaName =
+  llvm::StringSwitch(PragmaNameLoc->Ident->getName())
+  .Cases("unroll", "nounroll", "unroll_and_jam", "nounroll_and_jam",
+ PragmaNameLoc->Ident->getName())
+  .Default("clang loop");
+
   if (St->getStmtClass() != Stmt::DoStmtClass &&

[PATCH] D64471: Loop pragma parsing. NFC.

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer created this revision.
SjoerdMeijer added reviewers: Meinersbur, dmgreen, samparker.

I would like to add some pragma handling here, but couldn't resist a little NFC 
and tidy up first.


https://reviews.llvm.org/D64471

Files:
  clang/lib/Sema/SemaStmtAttr.cpp


Index: clang/lib/Sema/SemaStmtAttr.cpp
===
--- clang/lib/Sema/SemaStmtAttr.cpp
+++ clang/lib/Sema/SemaStmtAttr.cpp
@@ -82,22 +82,17 @@
   IdentifierLoc *StateLoc = A.getArgAsIdent(2);
   Expr *ValueExpr = A.getArgAsExpr(3);
 
-  bool PragmaUnroll = PragmaNameLoc->Ident->getName() == "unroll";
-  bool PragmaNoUnroll = PragmaNameLoc->Ident->getName() == "nounroll";
-  bool PragmaUnrollAndJam = PragmaNameLoc->Ident->getName() == 
"unroll_and_jam";
-  bool PragmaNoUnrollAndJam =
-  PragmaNameLoc->Ident->getName() == "nounroll_and_jam";
+  StringRef PragmaName =
+  llvm::StringSwitch(PragmaNameLoc->Ident->getName())
+  .Cases("unroll", "nounroll", "unroll_and_jam", "nounroll_and_jam",
+ PragmaNameLoc->Ident->getName())
+  .Default("clang loop");
+
   if (St->getStmtClass() != Stmt::DoStmtClass &&
   St->getStmtClass() != Stmt::ForStmtClass &&
   St->getStmtClass() != Stmt::CXXForRangeStmtClass &&
   St->getStmtClass() != Stmt::WhileStmtClass) {
-const char *Pragma =
-llvm::StringSwitch(PragmaNameLoc->Ident->getName())
-.Case("unroll", "#pragma unroll")
-.Case("nounroll", "#pragma nounroll")
-.Case("unroll_and_jam", "#pragma unroll_and_jam")
-.Case("nounroll_and_jam", "#pragma nounroll_and_jam")
-.Default("#pragma clang loop");
+std::string Pragma = "#pragma " + std::string(PragmaName);
 S.Diag(St->getBeginLoc(), diag::err_pragma_loop_precedes_nonloop) << 
Pragma;
 return nullptr;
   }
@@ -106,34 +101,29 @@
   LoopHintAttr::Spelling(A.getAttributeSpellingListIndex());
   LoopHintAttr::OptionType Option;
   LoopHintAttr::LoopHintState State;
-  if (PragmaNoUnroll) {
-// #pragma nounroll
-Option = LoopHintAttr::Unroll;
-State = LoopHintAttr::Disable;
-  } else if (PragmaUnroll) {
-if (ValueExpr) {
-  // #pragma unroll N
-  Option = LoopHintAttr::UnrollCount;
-  State = LoopHintAttr::Numeric;
-} else {
-  // #pragma unroll
-  Option = LoopHintAttr::Unroll;
-  State = LoopHintAttr::Enable;
-}
-  } else if (PragmaNoUnrollAndJam) {
-// #pragma nounroll_and_jam
-Option = LoopHintAttr::UnrollAndJam;
-State = LoopHintAttr::Disable;
-  } else if (PragmaUnrollAndJam) {
-if (ValueExpr) {
-  // #pragma unroll_and_jam N
-  Option = LoopHintAttr::UnrollAndJamCount;
-  State = LoopHintAttr::Numeric;
-} else {
-  // #pragma unroll_and_jam
-  Option = LoopHintAttr::UnrollAndJam;
-  State = LoopHintAttr::Enable;
-}
+
+  auto SetHints = [, ](LoopHintAttr::OptionType O,
+LoopHintAttr::LoopHintState S) {
+Option = O;
+State = S;
+  };
+
+  if (PragmaName == "nounroll") {
+SetHints(LoopHintAttr::Unroll, LoopHintAttr::Disable);
+  } else if (PragmaName == "unroll") {
+// #pragma unroll N
+if (ValueExpr)
+  SetHints(LoopHintAttr::UnrollCount, LoopHintAttr::Numeric);
+else
+  SetHints(LoopHintAttr::Unroll, LoopHintAttr::Enable);
+  } else if (PragmaName == "nounroll_and_jam") {
+SetHints(LoopHintAttr::UnrollAndJam, LoopHintAttr::Disable);
+  } else if (PragmaName == "unroll_and_jam") {
+// #pragma unroll_and_jam N
+if (ValueExpr)
+  SetHints(LoopHintAttr::UnrollAndJamCount, LoopHintAttr::Numeric);
+else
+  SetHints(LoopHintAttr::UnrollAndJam, LoopHintAttr::Enable);
   } else {
 // #pragma clang loop ...
 assert(OptionLoc && OptionLoc->Ident &&


Index: clang/lib/Sema/SemaStmtAttr.cpp
===
--- clang/lib/Sema/SemaStmtAttr.cpp
+++ clang/lib/Sema/SemaStmtAttr.cpp
@@ -82,22 +82,17 @@
   IdentifierLoc *StateLoc = A.getArgAsIdent(2);
   Expr *ValueExpr = A.getArgAsExpr(3);
 
-  bool PragmaUnroll = PragmaNameLoc->Ident->getName() == "unroll";
-  bool PragmaNoUnroll = PragmaNameLoc->Ident->getName() == "nounroll";
-  bool PragmaUnrollAndJam = PragmaNameLoc->Ident->getName() == "unroll_and_jam";
-  bool PragmaNoUnrollAndJam =
-  PragmaNameLoc->Ident->getName() == "nounroll_and_jam";
+  StringRef PragmaName =
+  llvm::StringSwitch(PragmaNameLoc->Ident->getName())
+  .Cases("unroll", "nounroll", "unroll_and_jam", "nounroll_and_jam",
+ PragmaNameLoc->Ident->getName())
+  .Default("clang loop");
+
   if (St->getStmtClass() != Stmt::DoStmtClass &&
   St->getStmtClass() != Stmt::ForStmtClass &&
   St->getStmtClass() != Stmt::CXXForRangeStmtClass &&
   St->getStmtClass() != Stmt::WhileStmtClass) {
-const char *Pragma =
-

[PATCH] D63936: [ARM] Minor fixes in command line option parsing

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer added a comment.

I will let Oliver finish the review (I am off for a few days), just some 
drive-by comments.




Comment at: llvm/lib/Support/ARMTargetParser.cpp:412
 
-  if (Extensions & AEK_CRC)
-Features.push_back("+crc");
-  else
-Features.push_back("-crc");
-
-  if (Extensions & AEK_DSP)
-Features.push_back("+dsp");
-  else
-Features.push_back("-dsp");
-
-  if (Extensions & AEK_FP16FML)
-Features.push_back("+fp16fml");
-  else
-Features.push_back("-fp16fml");
-
-  if (Extensions & AEK_RAS)
-Features.push_back("+ras");
-  else
-Features.push_back("-ras");
-
-  if (Extensions & AEK_DOTPROD)
-Features.push_back("+dotprod");
-  else
-Features.push_back("-dotprod");
+  for (const auto AE : ARCHExtNames) {
+if ((Extensions & AE.ID) == AE.ID && AE.Feature)

This could be a little local helper function, share the code, as exactly the 
same is done in `ARM::appendArchExtFeatures`



Comment at: llvm/unittests/Support/TargetParserTest.cpp:574
 
-  Extensions[ARM::AEK_CRC]= { "+crc",   "-crc" };
-  Extensions[ARM::AEK_DSP]= { "+dsp",   "-dsp" };
+  for (auto  : ARM::ARCHExtNames) {
+if (Ext.Feature && Ext.NegFeature)

I like this  approach.



Comment at: llvm/unittests/Support/TargetParserTest.cpp:580
+
   Extensions[ARM::AEK_HWDIVARM]   = { "+hwdiv-arm", "-hwdiv-arm" };
   Extensions[ARM::AEK_HWDIVTHUMB] = { "+hwdiv", "-hwdiv" };

but the fact that we have these still here, I guess that means they are not 
present in the table. Can we add them too? I guess that's why you've added 
`fp.dp`.



Comment at: llvm/unittests/Support/TargetParserTest.cpp:585
 
-  EXPECT_FALSE(AArch64::getExtensionFeatures(ARM::AEK_INVALID, Features));
+  EXPECT_FALSE(ARM::getExtensionFeatures(ARM::AEK_INVALID, Features));
 

Oops, thanks for fixing! :-)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63936/new/

https://reviews.llvm.org/D63936



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D60710: [ARM] Add ACLE feature macros for MVE.

[Sjoerd Meijer via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL362806: [ARM] Add ACLE feature macros for MVE (authored by 
SjoerdMeijer, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D60710?vs=203555=203561#toc

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60710/new/

https://reviews.llvm.org/D60710

Files:
  cfe/trunk/lib/Basic/Targets/ARM.cpp
  cfe/trunk/lib/Basic/Targets/ARM.h
  cfe/trunk/test/Preprocessor/arm-target-features.c

Index: cfe/trunk/test/Preprocessor/arm-target-features.c
===
--- cfe/trunk/test/Preprocessor/arm-target-features.c
+++ cfe/trunk/test/Preprocessor/arm-target-features.c
@@ -748,6 +748,27 @@
 // KRAIT-ALLOW-FP-INSTR:#define __ARM_FEATURE_DSP 1
 // KRAIT-ALLOW-FP-INSTR:#define  __ARM_VFPV4__ 1
 
+// RUN: %clang -target arm-arm-none-eabi -march=armv8.1-m.main -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-V81M %s
+// CHECK-V81M: #define __ARM_ARCH 8
+// CHECK-V81M: #define __ARM_ARCH_8_1M_MAIN__ 1
+// CHECK-V81M: #define __ARM_ARCH_ISA_THUMB 2
+// CHECK-V81M: #define __ARM_ARCH_PROFILE 'M'
+// CHECK-V81M-NOT: #define __ARM_FEATURE_DSP
+// CHECK-V81M-NOT: #define __ARM_FEATURE_MVE
+// CHECK-V81M-NOT: #define __ARM_FEATURE_SIMD32
+
+// RUN: %clang -target arm-arm-none-eabi -march=armv8.1-m.main+mve -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-V81M-MVE %s
+// CHECK-V81M-MVE: #define __ARM_FEATURE_DSP 1
+// CHECK-V81M-MVE: #define __ARM_FEATURE_MVE 1
+// CHECK-V81M-MVE: #define __ARM_FEATURE_SIMD32 1
+
+// RUN: %clang -target arm-arm-none-eabi -march=armv8.1-m.main+mve.fp -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-V81M-MVE-FP %s
+// CHECK-V81M-MVE-FP: #define __ARM_FEATURE_DSP 1
+// CHECK-V81M-MVE-FP: #define __ARM_FEATURE_FP16_SCALAR_ARITHMETIC 1
+// CHECK-V81M-MVE-FP: #define __ARM_FEATURE_MVE 3
+// CHECK-V81M-MVE-FP: #define __ARM_FEATURE_SIMD32 1
+// CHECK-V81M-MVE-FP: #define __ARM_FPV5__ 1
+
 // RUN: %clang -target armv8.1a-none-none-eabi -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-V81A %s
 // CHECK-V81A: #define __ARM_ARCH 8
 // CHECK-V81A: #define __ARM_ARCH_8_1A__ 1
Index: cfe/trunk/lib/Basic/Targets/ARM.h
===
--- cfe/trunk/lib/Basic/Targets/ARM.h
+++ cfe/trunk/lib/Basic/Targets/ARM.h
@@ -33,6 +33,11 @@
 FPARMV8 = (1 << 4)
   };
 
+  enum MVEMode {
+  MVE_INT = (1 << 0),
+  MVE_FP  = (1 << 1)
+  };
+
   // Possible HWDiv features.
   enum HWDivMode { HWDivThumb = (1 << 0), HWDivARM = (1 << 1) };
 
@@ -56,6 +61,7 @@
   unsigned ArchVersion;
 
   unsigned FPU : 5;
+  unsigned MVE : 2;
 
   unsigned IsAAPCS : 1;
   unsigned HWDiv : 2;
@@ -100,6 +106,8 @@
   bool isThumb() const;
   bool supportsThumb() const;
   bool supportsThumb2() const;
+  bool hasMVE() const;
+  bool hasMVEFloat() const;
 
   StringRef getCPUAttr() const;
   StringRef getCPUProfile() const;
Index: cfe/trunk/lib/Basic/Targets/ARM.cpp
===
--- cfe/trunk/lib/Basic/Targets/ARM.cpp
+++ cfe/trunk/lib/Basic/Targets/ARM.cpp
@@ -146,6 +146,14 @@
   }
 }
 
+bool ARMTargetInfo::hasMVE() const {
+  return ArchKind == llvm::ARM::ArchKind::ARMV8_1MMainline && MVE != 0;
+}
+
+bool ARMTargetInfo::hasMVEFloat() const {
+  return hasMVE() && (MVE & MVE_FP);
+}
+
 bool ARMTargetInfo::isThumb() const {
   return ArchISA == llvm::ARM::ISAKind::THUMB;
 }
@@ -460,6 +468,15 @@
   HasLegalHalfType = true;
 } else if (Feature == "+dotprod") {
   DotProd = true;
+} else if (Feature == "+mve") {
+  DSP = 1;
+  MVE |= MVE_INT;
+} else if (Feature == "+mve.fp") {
+  DSP = 1;
+  HasLegalHalfType = true;
+  FPU |= FPARMV8;
+  MVE |= MVE_INT | MVE_FP;
+  HW_FP |= HW_FP_SP | HW_FP_HP;
 }
   }
 
@@ -510,6 +527,7 @@
   .Case("vfp", FPU && !SoftFloat)
   .Case("hwdiv", HWDiv & HWDivThumb)
   .Case("hwdiv-arm", HWDiv & HWDivARM)
+  .Case("mve", hasMVE())
   .Default(false);
 }
 
@@ -725,6 +743,10 @@
 "0x" + Twine::utohexstr(HW_FP & ~HW_FP_DP));
   }
 
+  if (hasMVE()) {
+Builder.defineMacro("__ARM_FEATURE_MVE", hasMVEFloat() ? "3" : "1");
+  }
+
   Builder.defineMacro("__ARM_SIZEOF_WCHAR_T",
   Twine(Opts.WCharSize ? Opts.WCharSize : 4));
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D60710: [ARM] Add ACLE feature macros for MVE.

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer added a comment.

Thanks for reviewing!


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60710/new/

https://reviews.llvm.org/D60710



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D60710: [ARM] Add ACLE feature macros for MVE.

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer updated this revision to Diff 203555.
SjoerdMeijer added a comment.

Yep, sorry, missed that one.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60710/new/

https://reviews.llvm.org/D60710

Files:
  clang/lib/Basic/Targets/ARM.cpp
  clang/lib/Basic/Targets/ARM.h
  clang/test/Preprocessor/arm-target-features.c

Index: clang/test/Preprocessor/arm-target-features.c
===
--- clang/test/Preprocessor/arm-target-features.c
+++ clang/test/Preprocessor/arm-target-features.c
@@ -748,6 +748,27 @@
 // KRAIT-ALLOW-FP-INSTR:#define __ARM_FEATURE_DSP 1
 // KRAIT-ALLOW-FP-INSTR:#define  __ARM_VFPV4__ 1
 
+// RUN: %clang -target arm-arm-none-eabi -march=armv8.1-m.main -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-V81M %s
+// CHECK-V81M: #define __ARM_ARCH 8
+// CHECK-V81M: #define __ARM_ARCH_8_1M_MAIN__ 1
+// CHECK-V81M: #define __ARM_ARCH_ISA_THUMB 2
+// CHECK-V81M: #define __ARM_ARCH_PROFILE 'M'
+// CHECK-V81M-NOT: #define __ARM_FEATURE_DSP
+// CHECK-V81M-NOT: #define __ARM_FEATURE_MVE
+// CHECK-V81M-NOT: #define __ARM_FEATURE_SIMD32
+
+// RUN: %clang -target arm-arm-none-eabi -march=armv8.1-m.main+mve -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-V81M-MVE %s
+// CHECK-V81M-MVE: #define __ARM_FEATURE_DSP 1
+// CHECK-V81M-MVE: #define __ARM_FEATURE_MVE 1
+// CHECK-V81M-MVE: #define __ARM_FEATURE_SIMD32 1
+
+// RUN: %clang -target arm-arm-none-eabi -march=armv8.1-m.main+mve.fp -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-V81M-MVE-FP %s
+// CHECK-V81M-MVE-FP: #define __ARM_FEATURE_DSP 1
+// CHECK-V81M-MVE-FP: #define __ARM_FEATURE_FP16_SCALAR_ARITHMETIC 1
+// CHECK-V81M-MVE-FP: #define __ARM_FEATURE_MVE 3
+// CHECK-V81M-MVE-FP: #define __ARM_FEATURE_SIMD32 1
+// CHECK-V81M-MVE-FP: #define __ARM_FPV5__ 1
+
 // RUN: %clang -target armv8.1a-none-none-eabi -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-V81A %s
 // CHECK-V81A: #define __ARM_ARCH 8
 // CHECK-V81A: #define __ARM_ARCH_8_1A__ 1
Index: clang/lib/Basic/Targets/ARM.h
===
--- clang/lib/Basic/Targets/ARM.h
+++ clang/lib/Basic/Targets/ARM.h
@@ -33,6 +33,11 @@
 FPARMV8 = (1 << 4)
   };
 
+  enum MVEMode {
+  MVE_INT = (1 << 0),
+  MVE_FP  = (1 << 1)
+  };
+
   // Possible HWDiv features.
   enum HWDivMode { HWDivThumb = (1 << 0), HWDivARM = (1 << 1) };
 
@@ -56,6 +61,7 @@
   unsigned ArchVersion;
 
   unsigned FPU : 5;
+  unsigned MVE : 2;
 
   unsigned IsAAPCS : 1;
   unsigned HWDiv : 2;
@@ -100,6 +106,8 @@
   bool isThumb() const;
   bool supportsThumb() const;
   bool supportsThumb2() const;
+  bool hasMVE() const;
+  bool hasMVEFloat() const;
 
   StringRef getCPUAttr() const;
   StringRef getCPUProfile() const;
Index: clang/lib/Basic/Targets/ARM.cpp
===
--- clang/lib/Basic/Targets/ARM.cpp
+++ clang/lib/Basic/Targets/ARM.cpp
@@ -146,6 +146,14 @@
   }
 }
 
+bool ARMTargetInfo::hasMVE() const {
+  return ArchKind == llvm::ARM::ArchKind::ARMV8_1MMainline && MVE != 0;
+}
+
+bool ARMTargetInfo::hasMVEFloat() const {
+  return hasMVE() && (MVE & MVE_FP);
+}
+
 bool ARMTargetInfo::isThumb() const {
   return ArchISA == llvm::ARM::ISAKind::THUMB;
 }
@@ -448,6 +456,15 @@
   HasLegalHalfType = true;
 } else if (Feature == "+dotprod") {
   DotProd = true;
+} else if (Feature == "+mve") {
+  DSP = 1;
+  MVE |= MVE_INT;
+} else if (Feature == "+mve.fp") {
+  DSP = 1;
+  HasLegalHalfType = true;
+  FPU |= FPARMV8;
+  MVE |= MVE_INT | MVE_FP;
+  HW_FP |= HW_FP_SP | HW_FP_HP;
 }
   }
 
@@ -498,6 +515,7 @@
   .Case("vfp", FPU && !SoftFloat)
   .Case("hwdiv", HWDiv & HWDivThumb)
   .Case("hwdiv-arm", HWDiv & HWDivARM)
+  .Case("mve", hasMVE())
   .Default(false);
 }
 
@@ -713,6 +731,10 @@
 "0x" + Twine::utohexstr(HW_FP & ~HW_FP_DP));
   }
 
+  if (hasMVE()) {
+Builder.defineMacro("__ARM_FEATURE_MVE", hasMVEFloat() ? "3" : "1");
+  }
+
   Builder.defineMacro("__ARM_SIZEOF_WCHAR_T",
   Twine(Opts.WCharSize ? Opts.WCharSize : 4));
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D62998: [ARM] Fix bugs introduced by the fp64/d32 rework.

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer added a comment.

Cheers, LGTM too.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62998/new/

https://reviews.llvm.org/D62998



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D60710: [ARM] Add ACLE feature macros for MVE.

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer updated this revision to Diff 203522.
SjoerdMeijer added a comment.

Set FullFP16 for +mve.fp


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60710/new/

https://reviews.llvm.org/D60710

Files:
  clang/lib/Basic/Targets/ARM.cpp
  clang/lib/Basic/Targets/ARM.h
  clang/test/Preprocessor/arm-target-features.c

Index: clang/test/Preprocessor/arm-target-features.c
===
--- clang/test/Preprocessor/arm-target-features.c
+++ clang/test/Preprocessor/arm-target-features.c
@@ -748,6 +748,26 @@
 // KRAIT-ALLOW-FP-INSTR:#define __ARM_FEATURE_DSP 1
 // KRAIT-ALLOW-FP-INSTR:#define  __ARM_VFPV4__ 1
 
+// RUN: %clang -target arm-arm-none-eabi -march=armv8.1-m.main -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-V81M %s
+// CHECK-V81M: #define __ARM_ARCH 8
+// CHECK-V81M: #define __ARM_ARCH_8_1M_MAIN__ 1
+// CHECK-V81M: #define __ARM_ARCH_ISA_THUMB 2
+// CHECK-V81M: #define __ARM_ARCH_PROFILE 'M'
+// CHECK-V81M-NOT: #define __ARM_FEATURE_DSP
+// CHECK-V81M-NOT: #define __ARM_FEATURE_MVE
+// CHECK-V81M-NOT: #define __ARM_FEATURE_SIMD32
+
+// RUN: %clang -target arm-arm-none-eabi -march=armv8.1-m.main+mve -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-V81M-MVE %s
+// CHECK-V81M-MVE: #define __ARM_FEATURE_DSP 1
+// CHECK-V81M-MVE: #define __ARM_FEATURE_MVE 1
+// CHECK-V81M-MVE: #define __ARM_FEATURE_SIMD32 1
+
+// RUN: %clang -target arm-arm-none-eabi -march=armv8.1-m.main+mve.fp -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-V81M-MVE-FP %s
+// CHECK-V81M-MVE-FP: #define __ARM_FEATURE_DSP 1
+// CHECK-V81M-MVE-FP: #define __ARM_FEATURE_FP16_SCALAR_ARITHMETIC 1
+// CHECK-V81M-MVE-FP: #define __ARM_FEATURE_MVE 3
+// CHECK-V81M-MVE-FP: #define __ARM_FEATURE_SIMD32 1
+
 // RUN: %clang -target armv8.1a-none-none-eabi -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-V81A %s
 // CHECK-V81A: #define __ARM_ARCH 8
 // CHECK-V81A: #define __ARM_ARCH_8_1A__ 1
Index: clang/lib/Basic/Targets/ARM.h
===
--- clang/lib/Basic/Targets/ARM.h
+++ clang/lib/Basic/Targets/ARM.h
@@ -33,6 +33,11 @@
 FPARMV8 = (1 << 4)
   };
 
+  enum MVEMode {
+  MVE_INT = (1 << 0),
+  MVE_FP  = (1 << 1)
+  };
+
   // Possible HWDiv features.
   enum HWDivMode { HWDivThumb = (1 << 0), HWDivARM = (1 << 1) };
 
@@ -56,6 +61,7 @@
   unsigned ArchVersion;
 
   unsigned FPU : 5;
+  unsigned MVE : 2;
 
   unsigned IsAAPCS : 1;
   unsigned HWDiv : 2;
@@ -100,6 +106,8 @@
   bool isThumb() const;
   bool supportsThumb() const;
   bool supportsThumb2() const;
+  bool hasMVE() const;
+  bool hasMVEFloat() const;
 
   StringRef getCPUAttr() const;
   StringRef getCPUProfile() const;
Index: clang/lib/Basic/Targets/ARM.cpp
===
--- clang/lib/Basic/Targets/ARM.cpp
+++ clang/lib/Basic/Targets/ARM.cpp
@@ -146,6 +146,14 @@
   }
 }
 
+bool ARMTargetInfo::hasMVE() const {
+  return ArchKind == llvm::ARM::ArchKind::ARMV8_1MMainline && MVE != 0;
+}
+
+bool ARMTargetInfo::hasMVEFloat() const {
+  return hasMVE() && (MVE & MVE_FP);
+}
+
 bool ARMTargetInfo::isThumb() const {
   return ArchISA == llvm::ARM::ISAKind::THUMB;
 }
@@ -448,6 +456,14 @@
   HasLegalHalfType = true;
 } else if (Feature == "+dotprod") {
   DotProd = true;
+} else if (Feature == "+mve") {
+  DSP = 1;
+  MVE |= MVE_INT;
+} else if (Feature == "+mve.fp") {
+  DSP = 1;
+  HasLegalHalfType = true;
+  MVE |= MVE_INT | MVE_FP;
+  HW_FP |= HW_FP_SP | HW_FP_HP;
 }
   }
 
@@ -498,6 +514,7 @@
   .Case("vfp", FPU && !SoftFloat)
   .Case("hwdiv", HWDiv & HWDivThumb)
   .Case("hwdiv-arm", HWDiv & HWDivARM)
+  .Case("mve", hasMVE())
   .Default(false);
 }
 
@@ -713,6 +730,10 @@
 "0x" + Twine::utohexstr(HW_FP & ~HW_FP_DP));
   }
 
+  if (hasMVE()) {
+Builder.defineMacro("__ARM_FEATURE_MVE", hasMVEFloat() ? "3" : "1");
+  }
+
   Builder.defineMacro("__ARM_SIZEOF_WCHAR_T",
   Twine(Opts.WCharSize ? Opts.WCharSize : 4));
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D60710: [ARM] Add ACLE feature macros for MVE.

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer marked an inline comment as done.
SjoerdMeijer added inline comments.



Comment at: clang/lib/Basic/Targets/ARM.cpp:465
+  MVE |= MVE_INT | MVE_FP;
+  HW_FP |= HW_FP_SP | HW_FP_HP;
 }

ostannard wrote:
> Does this also need to set FPU and HasLegalHalfType?
Yep, thanks for catching this one, will fix this.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60710/new/

https://reviews.llvm.org/D60710



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D62998: [ARM] Fix bugs introduced by the fp64/d32 rework.

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer added inline comments.



Comment at: clang/lib/Driver/ToolChains/Arch/ARM.cpp:103
  std::vector ,
+ std::vector ,
  const llvm::Triple ) {

Nitpick: can you comment in code somewhere what the purpose is of 
`FeaturesAfter`? Can it be renamed to something more descriptive? I don't know 
what it could be to be honest...



Comment at: clang/lib/Driver/ToolChains/Arch/ARM.cpp:449
 //as well.
-for (std::string Feature : {"vfp2", "vfp3", "vfp4", "fp-armv8", "fullfp16",
-"neon", "crypto", "dotprod", "fp16fml"})
-  if (std::find(std::begin(Features), std::end(Features), "+" + Feature) 
!= std::end(Features))
-Features.push_back(Args.MakeArgString("-" + Feature));
-
-// Disable the base feature unconditionally, even if it was not
-// explicitly in the features list (e.g. if we had +vfp3, which
-// implies it).
-Features.push_back("-fpregs");
+for (std::string Feature : {
+"vfp2", "vfp2sp", "vfp2d16", "vfp2d16sp",

This function and this code is truly one of the worst I guess I mean, it's 
not worse than it was before, but you'd really hope that there was a 
(targetparser) API that gives you a list of FPUs, possibly matching some 
criteria, rather than yet another list of hard coded strings. It gets worse, 
this is repeated in `lib/Support/ARMTargetParser.cpp`. End of rant! ;-)

More on topic: do we need to add +mve.fp here?



Comment at: llvm/lib/Support/ARMTargetParser.cpp:178
+
+{"+fpregs", "-fpregs", FPUVersion::VFPV2, FPURestriction::SP_D16},
+{"+vfp2", "-vfp2", FPUVersion::VFPV2, FPURestriction::None},

This looks familiar  I don't think we can easily simplify this? Or get rid 
of? I guess that's best done in a follow up when we this working again? Perhaps 
the only insignificant simplification is that we don't need the + and 
- because they are the same.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62998/new/

https://reviews.llvm.org/D62998



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D60710: [ARM] Add ACLE feature macros for MVE.

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer updated this revision to Diff 203507.
SjoerdMeijer added a comment.

Added tests


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60710/new/

https://reviews.llvm.org/D60710

Files:
  clang/lib/Basic/Targets/ARM.cpp
  clang/lib/Basic/Targets/ARM.h
  clang/test/Preprocessor/arm-target-features.c

Index: clang/test/Preprocessor/arm-target-features.c
===
--- clang/test/Preprocessor/arm-target-features.c
+++ clang/test/Preprocessor/arm-target-features.c
@@ -748,6 +748,25 @@
 // KRAIT-ALLOW-FP-INSTR:#define __ARM_FEATURE_DSP 1
 // KRAIT-ALLOW-FP-INSTR:#define  __ARM_VFPV4__ 1
 
+// RUN: %clang -target arm-arm-none-eabi -march=armv8.1-m.main -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-V81M %s
+// CHECK-V81M: #define __ARM_ARCH 8
+// CHECK-V81M: #define __ARM_ARCH_8_1M_MAIN__ 1
+// CHECK-V81M: #define __ARM_ARCH_ISA_THUMB 2
+// CHECK-V81M: #define __ARM_ARCH_PROFILE 'M'
+// CHECK-V81M-NOT: #define __ARM_FEATURE_DSP
+// CHECK-V81M-NOT: #define __ARM_FEATURE_MVE
+// CHECK-V81M-NOT: #define __ARM_FEATURE_SIMD32
+
+// RUN: %clang -target arm-arm-none-eabi -march=armv8.1-m.main+mve -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-V81M-MVE %s
+// CHECK-V81M-MVE: #define __ARM_FEATURE_DSP 1
+// CHECK-V81M-MVE: #define __ARM_FEATURE_MVE 1
+// CHECK-V81M-MVE: #define __ARM_FEATURE_SIMD32 1
+
+// RUN: %clang -target arm-arm-none-eabi -march=armv8.1-m.main+mve.fp -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-V81M-MVE-FP %s
+// CHECK-V81M-MVE-FP: #define __ARM_FEATURE_DSP 1
+// CHECK-V81M-MVE-FP: #define __ARM_FEATURE_MVE 3
+// CHECK-V81M-MVE-FP: #define __ARM_FEATURE_SIMD32 1
+
 // RUN: %clang -target armv8.1a-none-none-eabi -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-V81A %s
 // CHECK-V81A: #define __ARM_ARCH 8
 // CHECK-V81A: #define __ARM_ARCH_8_1A__ 1
Index: clang/lib/Basic/Targets/ARM.h
===
--- clang/lib/Basic/Targets/ARM.h
+++ clang/lib/Basic/Targets/ARM.h
@@ -33,6 +33,11 @@
 FPARMV8 = (1 << 4)
   };
 
+  enum MVEMode {
+  MVE_INT = (1 << 0),
+  MVE_FP  = (1 << 1)
+  };
+
   // Possible HWDiv features.
   enum HWDivMode { HWDivThumb = (1 << 0), HWDivARM = (1 << 1) };
 
@@ -56,6 +61,7 @@
   unsigned ArchVersion;
 
   unsigned FPU : 5;
+  unsigned MVE : 2;
 
   unsigned IsAAPCS : 1;
   unsigned HWDiv : 2;
@@ -100,6 +106,8 @@
   bool isThumb() const;
   bool supportsThumb() const;
   bool supportsThumb2() const;
+  bool hasMVE() const;
+  bool hasMVEFloat() const;
 
   StringRef getCPUAttr() const;
   StringRef getCPUProfile() const;
Index: clang/lib/Basic/Targets/ARM.cpp
===
--- clang/lib/Basic/Targets/ARM.cpp
+++ clang/lib/Basic/Targets/ARM.cpp
@@ -146,6 +146,14 @@
   }
 }
 
+bool ARMTargetInfo::hasMVE() const {
+  return ArchKind == llvm::ARM::ArchKind::ARMV8_1MMainline && MVE != 0;
+}
+
+bool ARMTargetInfo::hasMVEFloat() const {
+  return hasMVE() && (MVE & MVE_FP);
+}
+
 bool ARMTargetInfo::isThumb() const {
   return ArchISA == llvm::ARM::ISAKind::THUMB;
 }
@@ -448,6 +456,13 @@
   HasLegalHalfType = true;
 } else if (Feature == "+dotprod") {
   DotProd = true;
+} else if (Feature == "+mve") {
+  DSP = 1;
+  MVE |= MVE_INT;
+} else if (Feature == "+mve.fp") {
+  DSP = 1;
+  MVE |= MVE_INT | MVE_FP;
+  HW_FP |= HW_FP_SP | HW_FP_HP;
 }
   }
 
@@ -498,6 +513,7 @@
   .Case("vfp", FPU && !SoftFloat)
   .Case("hwdiv", HWDiv & HWDivThumb)
   .Case("hwdiv-arm", HWDiv & HWDivARM)
+  .Case("mve", hasMVE())
   .Default(false);
 }
 
@@ -713,6 +729,10 @@
 "0x" + Twine::utohexstr(HW_FP & ~HW_FP_DP));
   }
 
+  if (hasMVE()) {
+Builder.defineMacro("__ARM_FEATURE_MVE", hasMVEFloat() ? "3" : "1");
+  }
+
   Builder.defineMacro("__ARM_SIZEOF_WCHAR_T",
   Twine(Opts.WCharSize ? Opts.WCharSize : 4));
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D60697: [ARM] Allow "-march=foo+fp" to vary with foo.

[Sjoerd Meijer via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL362600: [ARM] Allow -march=foo+fp to vary with 
foo (authored by SjoerdMeijer, committed by ).
Herald added a subscriber: kristina.

Changed prior to commit:
  https://reviews.llvm.org/D60697?vs=202934=203129#toc

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60697/new/

https://reviews.llvm.org/D60697

Files:
  llvm/trunk/include/llvm/Support/ARMTargetParser.h
  llvm/trunk/lib/Support/ARMTargetParser.cpp

Index: llvm/trunk/include/llvm/Support/ARMTargetParser.h
===
--- llvm/trunk/include/llvm/Support/ARMTargetParser.h
+++ llvm/trunk/include/llvm/Support/ARMTargetParser.h
@@ -240,6 +240,8 @@
 StringRef getSubArch(ArchKind AK);
 StringRef getArchExtName(unsigned ArchExtKind);
 StringRef getArchExtFeature(StringRef ArchExt);
+bool appendArchExtFeatures(StringRef CPU, ARM::ArchKind AK, StringRef ArchExt,
+   std::vector );
 StringRef getHWDivName(unsigned HWDivKind);
 
 // Information by Name
Index: llvm/trunk/lib/Support/ARMTargetParser.cpp
===
--- llvm/trunk/lib/Support/ARMTargetParser.cpp
+++ llvm/trunk/lib/Support/ARMTargetParser.cpp
@@ -485,22 +485,85 @@
   return StringRef();
 }
 
-StringRef ARM::getArchExtFeature(StringRef ArchExt) {
-  if (ArchExt.startswith("no")) {
-StringRef ArchExtBase(ArchExt.substr(2));
-for (const auto AE : ARCHExtNames) {
-  if (AE.NegFeature && ArchExtBase == AE.getName())
-return StringRef(AE.NegFeature);
-}
+static bool stripNegationPrefix(StringRef ) {
+  if (Name.startswith("no")) {
+Name = Name.substr(2);
+return true;
   }
+  return false;
+}
+
+StringRef ARM::getArchExtFeature(StringRef ArchExt) {
+  bool Negated = stripNegationPrefix(ArchExt);
   for (const auto AE : ARCHExtNames) {
 if (AE.Feature && ArchExt == AE.getName())
-  return StringRef(AE.Feature);
+  return StringRef(Negated ? AE.NegFeature : AE.Feature);
   }
 
   return StringRef();
 }
 
+static unsigned findDoublePrecisionFPU(unsigned InputFPUKind) {
+  const ARM::FPUName  = ARM::FPUNames[InputFPUKind];
+
+  // If the input FPU already supports double-precision, then there
+  // isn't any different FPU we can return here.
+  //
+  // The current available FPURestriction values are None (no
+  // restriction), D16 (only 16 d-regs) and SP_D16 (16 d-regs
+  // and single precision only); there's no value representing
+  // SP restriction without D16. So this test just means 'is it
+  // SP only?'.
+  if (InputFPU.Restriction != ARM::FPURestriction::SP_D16)
+return ARM::FK_INVALID;
+
+  // Otherwise, look for an FPU entry with all the same fields, except
+  // that SP_D16 has been replaced with just D16, representing adding
+  // double precision and not changing anything else.
+  for (const ARM::FPUName  : ARM::FPUNames) {
+if (CandidateFPU.FPUVer == InputFPU.FPUVer &&
+CandidateFPU.NeonSupport == InputFPU.NeonSupport &&
+CandidateFPU.Restriction == ARM::FPURestriction::D16) {
+  return CandidateFPU.ID;
+}
+  }
+
+  // nothing found
+  return ARM::FK_INVALID;
+}
+
+bool ARM::appendArchExtFeatures(
+  StringRef CPU, ARM::ArchKind AK, StringRef ArchExt,
+  std::vector ) {
+  StringRef StandardFeature = getArchExtFeature(ArchExt);
+  if (!StandardFeature.empty()) {
+Features.push_back(StandardFeature);
+return true;
+  }
+
+  const bool Negated = stripNegationPrefix(ArchExt);
+
+  if (CPU == "")
+CPU = "generic";
+
+  if (ArchExt == "fp" || ArchExt == "fp.dp") {
+unsigned FPUKind;
+if (ArchExt == "fp.dp") {
+  if (Negated) {
+Features.push_back("-fp64");
+return true;
+  }
+  FPUKind = findDoublePrecisionFPU(getDefaultFPU(CPU, AK));
+} else if (Negated) {
+  FPUKind = ARM::FK_NONE;
+} else {
+  FPUKind = getDefaultFPU(CPU, AK);
+}
+return ARM::getFPUFeatures(FPUKind, Features);
+  }
+  return false;
+}
+
 StringRef ARM::getHWDivName(unsigned HWDivKind) {
   for (const auto D : HWDivNames) {
 if (HWDivKind == D.ID)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D60697: [ARM] Allow "-march=foo+fp" to vary with foo.

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer updated this revision to Diff 202934.
SjoerdMeijer added a comment.

Hi Oliver, thanks for your comments!

This was the easy one, they have been added:

> I also don't see any tests for the negated forms of either feature.

The trouble begun with this:

> +fp.dp, but the FPU is already double-precision
>  +fp.dp, but no double-precision FPU exists (are there any FPUs which cause 
> this?)
>  +[no]fp or +[no]fp.dp for a CPU/arch which doesn't have any FPUs.

Because I found that basically none of this worked. The main reason was that we 
were always passing `generic`. To address that we at least have a chance of 
seeing a sensible CPU name, I have swapped the order of parsing -march and 
-mcpu. I.e., we parse -mcpu first, and pass that to `checkARMArchName`, which 
will eventually call `appendArchExtFeatures`. I think that makes more sense 
when we use the CPUname to query `getDefaultFPU`.

Then about the more fancy diagnostics (e.g. "fp.dp, but the FPU is already 
double-precision"): I've removed any attempt to throw clever diagnostics. I 
don't think, in general, that we provide this kind of service level. I.e., we 
need to do a lot more work here to avoid a meaningless, confusing, and thus 
useless  "--march=... not supported" error message when we provide +fp.dp on 
the -march when e.g. the CPU already enabled this.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60697/new/

https://reviews.llvm.org/D60697

Files:
  clang/lib/Driver/ToolChains/Arch/ARM.cpp
  clang/lib/Driver/ToolChains/Arch/ARM.h
  clang/test/Driver/armv8.1m.main.c
  clang/test/Driver/armv8.1m.main.s
  llvm/include/llvm/Support/ARMTargetParser.h
  llvm/lib/Support/ARMTargetParser.cpp

Index: llvm/lib/Support/ARMTargetParser.cpp
===
--- llvm/lib/Support/ARMTargetParser.cpp
+++ llvm/lib/Support/ARMTargetParser.cpp
@@ -484,22 +484,85 @@
   return StringRef();
 }
 
-StringRef ARM::getArchExtFeature(StringRef ArchExt) {
-  if (ArchExt.startswith("no")) {
-StringRef ArchExtBase(ArchExt.substr(2));
-for (const auto AE : ARCHExtNames) {
-  if (AE.NegFeature && ArchExtBase == AE.getName())
-return StringRef(AE.NegFeature);
-}
+static bool stripNegationPrefix(StringRef ) {
+  if (Name.startswith("no")) {
+Name = Name.substr(2);
+return true;
   }
+  return false;
+}
+
+StringRef ARM::getArchExtFeature(StringRef ArchExt) {
+  bool Negated = stripNegationPrefix(ArchExt);
   for (const auto AE : ARCHExtNames) {
 if (AE.Feature && ArchExt == AE.getName())
-  return StringRef(AE.Feature);
+  return StringRef(Negated ? AE.NegFeature : AE.Feature);
   }
 
   return StringRef();
 }
 
+static unsigned findDoublePrecisionFPU(unsigned InputFPUKind) {
+  const ARM::FPUName  = ARM::FPUNames[InputFPUKind];
+
+  // If the input FPU already supports double-precision, then there
+  // isn't any different FPU we can return here.
+  //
+  // The current available FPURestriction values are None (no
+  // restriction), D16 (only 16 d-regs) and SP_D16 (16 d-regs
+  // and single precision only); there's no value representing
+  // SP restriction without D16. So this test just means 'is it
+  // SP only?'.
+  if (InputFPU.Restriction != ARM::FPURestriction::SP_D16)
+return ARM::FK_INVALID;
+
+  // Otherwise, look for an FPU entry with all the same fields, except
+  // that SP_D16 has been replaced with just D16, representing adding
+  // double precision and not changing anything else.
+  for (const ARM::FPUName  : ARM::FPUNames) {
+if (CandidateFPU.FPUVer == InputFPU.FPUVer &&
+CandidateFPU.NeonSupport == InputFPU.NeonSupport &&
+CandidateFPU.Restriction == ARM::FPURestriction::D16) {
+  return CandidateFPU.ID;
+}
+  }
+
+  // nothing found
+  return ARM::FK_INVALID;
+}
+
+bool ARM::appendArchExtFeatures(
+  StringRef CPU, ARM::ArchKind AK, StringRef ArchExt,
+  std::vector ) {
+  StringRef StandardFeature = getArchExtFeature(ArchExt);
+  if (!StandardFeature.empty()) {
+Features.push_back(StandardFeature);
+return true;
+  }
+
+  const bool Negated = stripNegationPrefix(ArchExt);
+
+  if (CPU == "")
+CPU = "generic";
+
+  if (ArchExt == "fp" || ArchExt == "fp.dp") {
+unsigned FPUKind;
+if (ArchExt == "fp.dp") {
+  if (Negated) {
+Features.push_back("-fp64");
+return true;
+  }
+  FPUKind = findDoublePrecisionFPU(getDefaultFPU(CPU, AK));
+} else if (Negated) {
+  FPUKind = ARM::FK_NONE;
+} else {
+  FPUKind = getDefaultFPU(CPU, AK);
+}
+return ARM::getFPUFeatures(FPUKind, Features);
+  }
+  return false;
+}
+
 StringRef ARM::getHWDivName(unsigned HWDivKind) {
   for (const auto D : HWDivNames) {
 if (HWDivKind == D.ID)
Index: llvm/include/llvm/Support/ARMTargetParser.h
===
--- llvm/include/llvm/Support/ARMTargetParser.h
+++ llvm/include/llvm/Support/ARMTargetParser.h
@@ 

[PATCH] D60697: [ARM] Allow "-march=foo+fp" to vary with foo.

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer updated this revision to Diff 202433.
SjoerdMeijer added a comment.

This time with tests.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60697/new/

https://reviews.llvm.org/D60697

Files:
  clang/lib/Driver/ToolChains/Arch/ARM.cpp
  clang/lib/Driver/ToolChains/Arch/ARM.h
  clang/test/Driver/armv8.1m.main.c
  clang/test/Driver/armv8.1m.main.s
  llvm/include/llvm/Support/ARMTargetParser.h
  llvm/lib/Support/ARMTargetParser.cpp

Index: llvm/lib/Support/ARMTargetParser.cpp
===
--- llvm/lib/Support/ARMTargetParser.cpp
+++ llvm/lib/Support/ARMTargetParser.cpp
@@ -484,22 +484,100 @@
   return StringRef();
 }
 
-StringRef ARM::getArchExtFeature(StringRef ArchExt) {
-  if (ArchExt.startswith("no")) {
-StringRef ArchExtBase(ArchExt.substr(2));
-for (const auto AE : ARCHExtNames) {
-  if (AE.NegFeature && ArchExtBase == AE.getName())
-return StringRef(AE.NegFeature);
-}
+static bool stripNegationPrefix(StringRef ) {
+  if (Name.startswith("no")) {
+Name = Name.substr(2);
+return true;
   }
+  return false;
+}
+
+StringRef ARM::getArchExtFeature(StringRef ArchExt) {
+  bool Negated = stripNegationPrefix(ArchExt);
   for (const auto AE : ARCHExtNames) {
 if (AE.Feature && ArchExt == AE.getName())
-  return StringRef(AE.Feature);
+  return StringRef(Negated ? AE.NegFeature : AE.Feature);
   }
 
   return StringRef();
 }
 
+static unsigned findDoublePrecisionFPU(unsigned InputFPUKind) {
+  const ARM::FPUName  = ARM::FPUNames[InputFPUKind];
+
+  // If the input FPU already supports double-precision, then there
+  // isn't any different FPU we can return here.
+  //
+  // The current available FPURestriction values are None (no
+  // restriction), D16 (only 16 d-regs) and SP_D16 (16 d-regs
+  // and single precision only); there's no value representing
+  // SP restriction without D16. So this test just means 'is it
+  // SP only?'.
+  if (InputFPU.Restriction != ARM::FPURestriction::SP_D16)
+return ARM::FK_INVALID;
+
+  // Otherwise, look for an FPU entry with all the same fields, except
+  // that SP_D16 has been replaced with just D16, representing adding
+  // double precision and not changing anything else.
+  for (const ARM::FPUName  : ARM::FPUNames) {
+if (CandidateFPU.FPUVer == InputFPU.FPUVer &&
+CandidateFPU.NeonSupport == InputFPU.NeonSupport &&
+CandidateFPU.Restriction == ARM::FPURestriction::D16) {
+  return CandidateFPU.ID;
+}
+  }
+
+  // nothing found
+  return ARM::FK_INVALID;
+}
+
+bool ARM::appendArchExtFeatures(
+  StringRef CPU, ARM::ArchKind AK, StringRef ArchExt,
+  std::vector ) {
+  StringRef StandardFeature = getArchExtFeature(ArchExt);
+  if (!StandardFeature.empty()) {
+Features.push_back(StandardFeature);
+return true;
+  }
+
+  bool Negated = stripNegationPrefix(ArchExt);
+
+  if (ArchExt == "fp" || ArchExt == "fp.dp") {
+unsigned FPUKind;
+
+if (ArchExt == "fp.dp") {
+  unsigned DoubleFPUKind = findDoublePrecisionFPU(getDefaultFPU(CPU, AK));
+
+  // If the default FPU already supports double-precision, or if
+  // there is no double-prec FPU that extends it, then "fp.dp"
+  // doesn't have a separate meaning, and we treat it as an
+  // invalid extension name.
+  if (DoubleFPUKind == FK_INVALID)
+return false;
+
+  // If there _is_ a separate double-precision FPU, then "nofp.dp"
+  // should disable just the double-precision extension, leaving
+  // the base FPU still enabled if it previously was.
+  if (Negated) {
+Features.push_back("-fp64");
+return true;
+  }
+
+  // Otherwise, select the double-precision FPU.
+  FPUKind = DoubleFPUKind;
+} else if (Negated) {
+  FPUKind = ARM::FK_NONE;
+} else {
+  FPUKind = getDefaultFPU(CPU, AK);
+  if (FPUKind == ARM::FK_NONE)
+return false;
+}
+return ARM::getFPUFeatures(FPUKind, Features);
+  }
+
+  return false;
+}
+
 StringRef ARM::getHWDivName(unsigned HWDivKind) {
   for (const auto D : HWDivNames) {
 if (HWDivKind == D.ID)
Index: llvm/include/llvm/Support/ARMTargetParser.h
===
--- llvm/include/llvm/Support/ARMTargetParser.h
+++ llvm/include/llvm/Support/ARMTargetParser.h
@@ -240,6 +240,8 @@
 StringRef getSubArch(ArchKind AK);
 StringRef getArchExtName(unsigned ArchExtKind);
 StringRef getArchExtFeature(StringRef ArchExt);
+bool appendArchExtFeatures(StringRef CPU, ARM::ArchKind AK, StringRef ArchExt,
+   std::vector );
 StringRef getHWDivName(unsigned HWDivKind);
 
 // Information by Name
Index: clang/test/Driver/armv8.1m.main.s
===
--- clang/test/Driver/armv8.1m.main.s
+++ clang/test/Driver/armv8.1m.main.s
@@ -5,8 +5,14 @@
 # RUN:  FileCheck --check-prefix=ERROR-V81M < %t %s
 # RUN: not %clang -c 

[PATCH] D60697: [ARM] Allow "-march=foo+fp" to vary with foo.

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer marked an inline comment as done.
SjoerdMeijer added a comment.

Ah yes, the school boy error! ;-) Actually, there was a test, but in a 
different patch; I will move it to here.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60697/new/

https://reviews.llvm.org/D60697



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D60697: [ARM] Allow "-march=foo+fp" to vary with foo.

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer updated this revision to Diff 202388.
SjoerdMeijer added a comment.

This addresses @t.p.northover comment.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60697/new/

https://reviews.llvm.org/D60697

Files:
  clang/lib/Driver/ToolChains/Arch/ARM.cpp
  clang/lib/Driver/ToolChains/Arch/ARM.h
  llvm/include/llvm/Support/ARMTargetParser.h
  llvm/lib/Support/ARMTargetParser.cpp

Index: llvm/lib/Support/ARMTargetParser.cpp
===
--- llvm/lib/Support/ARMTargetParser.cpp
+++ llvm/lib/Support/ARMTargetParser.cpp
@@ -484,22 +484,100 @@
   return StringRef();
 }
 
-StringRef ARM::getArchExtFeature(StringRef ArchExt) {
-  if (ArchExt.startswith("no")) {
-StringRef ArchExtBase(ArchExt.substr(2));
-for (const auto AE : ARCHExtNames) {
-  if (AE.NegFeature && ArchExtBase == AE.getName())
-return StringRef(AE.NegFeature);
-}
+static bool stripNegationPrefix(StringRef ) {
+  if (Name.startswith("no")) {
+Name = Name.substr(2);
+return true;
   }
+  return false;
+}
+
+StringRef ARM::getArchExtFeature(StringRef ArchExt) {
+  bool Negated = stripNegationPrefix(ArchExt);
   for (const auto AE : ARCHExtNames) {
 if (AE.Feature && ArchExt == AE.getName())
-  return StringRef(AE.Feature);
+  return StringRef(Negated ? AE.NegFeature : AE.Feature);
   }
 
   return StringRef();
 }
 
+static unsigned findDoublePrecisionFPU(unsigned InputFPUKind) {
+  const ARM::FPUName  = ARM::FPUNames[InputFPUKind];
+
+  // If the input FPU already supports double-precision, then there
+  // isn't any different FPU we can return here.
+  //
+  // The current available FPURestriction values are None (no
+  // restriction), D16 (only 16 d-regs) and SP_D16 (16 d-regs
+  // and single precision only); there's no value representing
+  // SP restriction without D16. So this test just means 'is it
+  // SP only?'.
+  if (InputFPU.Restriction != ARM::FPURestriction::SP_D16)
+return ARM::FK_INVALID;
+
+  // Otherwise, look for an FPU entry with all the same fields, except
+  // that SP_D16 has been replaced with just D16, representing adding
+  // double precision and not changing anything else.
+  for (const ARM::FPUName  : ARM::FPUNames) {
+if (CandidateFPU.FPUVer == InputFPU.FPUVer &&
+CandidateFPU.NeonSupport == InputFPU.NeonSupport &&
+CandidateFPU.Restriction == ARM::FPURestriction::D16) {
+  return CandidateFPU.ID;
+}
+  }
+
+  // nothing found
+  return ARM::FK_INVALID;
+}
+
+bool ARM::appendArchExtFeatures(
+  StringRef CPU, ARM::ArchKind AK, StringRef ArchExt,
+  std::vector ) {
+  StringRef StandardFeature = getArchExtFeature(ArchExt);
+  if (!StandardFeature.empty()) {
+Features.push_back(StandardFeature);
+return true;
+  }
+
+  bool Negated = stripNegationPrefix(ArchExt);
+
+  if (ArchExt == "fp" || ArchExt == "fp.dp") {
+unsigned FPUKind;
+
+if (ArchExt == "fp.dp") {
+  unsigned DoubleFPUKind = findDoublePrecisionFPU(getDefaultFPU(CPU, AK));
+
+  // If the default FPU already supports double-precision, or if
+  // there is no double-prec FPU that extends it, then "fp.dp"
+  // doesn't have a separate meaning, and we treat it as an
+  // invalid extension name.
+  if (DoubleFPUKind == FK_INVALID)
+return false;
+
+  // If there _is_ a separate double-precision FPU, then "nofp.dp"
+  // should disable just the double-precision extension, leaving
+  // the base FPU still enabled if it previously was.
+  if (Negated) {
+Features.push_back("-fp64");
+return true;
+  }
+
+  // Otherwise, select the double-precision FPU.
+  FPUKind = DoubleFPUKind;
+} else if (Negated) {
+  FPUKind = ARM::FK_NONE;
+} else {
+  FPUKind = getDefaultFPU(CPU, AK);
+  if (FPUKind == ARM::FK_NONE)
+return false;
+}
+return ARM::getFPUFeatures(FPUKind, Features);
+  }
+
+  return false;
+}
+
 StringRef ARM::getHWDivName(unsigned HWDivKind) {
   for (const auto D : HWDivNames) {
 if (HWDivKind == D.ID)
Index: llvm/include/llvm/Support/ARMTargetParser.h
===
--- llvm/include/llvm/Support/ARMTargetParser.h
+++ llvm/include/llvm/Support/ARMTargetParser.h
@@ -240,6 +240,8 @@
 StringRef getSubArch(ArchKind AK);
 StringRef getArchExtName(unsigned ArchExtKind);
 StringRef getArchExtFeature(StringRef ArchExt);
+bool appendArchExtFeatures(StringRef CPU, ARM::ArchKind AK, StringRef ArchExt,
+   std::vector );
 StringRef getHWDivName(unsigned HWDivKind);
 
 // Information by Name
Index: clang/lib/Driver/ToolChains/Arch/ARM.h
===
--- clang/lib/Driver/ToolChains/Arch/ARM.h
+++ clang/lib/Driver/ToolChains/Arch/ARM.h
@@ -13,6 +13,7 @@
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/Triple.h"
 #include "llvm/Option/Option.h"
+#include 

[PATCH] D60699: [ARM] add CLI support for 8.1-M and MVE.

[Sjoerd Meijer via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL362096: [ARM] Add CLI support for Armv8.1-M and MVE 
(authored by SjoerdMeijer, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D60699?vs=195157=202185#toc

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60699/new/

https://reviews.llvm.org/D60699

Files:
  cfe/trunk/lib/Basic/Targets/ARM.cpp
  cfe/trunk/test/Driver/armv8.1m.main.c
  cfe/trunk/test/Driver/armv8.1m.main.s

Index: cfe/trunk/lib/Basic/Targets/ARM.cpp
===
--- cfe/trunk/lib/Basic/Targets/ARM.cpp
+++ cfe/trunk/lib/Basic/Targets/ARM.cpp
@@ -197,6 +197,8 @@
 return "8M_MAIN";
   case llvm::ARM::ArchKind::ARMV8R:
 return "8R";
+  case llvm::ARM::ArchKind::ARMV8_1MMainline:
+return "8_1M_MAIN";
   }
 }
 
Index: cfe/trunk/test/Driver/armv8.1m.main.s
===
--- cfe/trunk/test/Driver/armv8.1m.main.s
+++ cfe/trunk/test/Driver/armv8.1m.main.s
@@ -0,0 +1,65 @@
+# RUN: not %clang -c -target arm-none-none-eabi -march=armv8-m.main %s 2>%t
+# RUN:  FileCheck --check-prefix=ERROR-V8M < %t %s
+# RUN: not %clang -c -target arm-none-none-eabi -march=armv8.1-m.main %s 2>%t
+# RUN:  FileCheck --check-prefix=ERROR-V81M < %t %s
+# RUN: not %clang -c -target arm-none-none-eabi -march=armv8.1-m.main+dsp %s 2>%t
+# RUN:  FileCheck --check-prefix=ERROR-V81M_DSP < %t %s
+# RUN: not %clang -c -target arm-none-none-eabi -march=armv8.1-m.main+fp %s 2>%t
+# RUN:  FileCheck --check-prefix=ERROR-V81M_FP < %t %s
+# RUN: not %clang -c -target arm-none-none-eabi -march=armv8.1-m.main+fp.dp %s 2>%t
+# RUN:  FileCheck --check-prefix=ERROR-V81M_FPDP < %t %s
+# RUN: not %clang -c -target arm-none-none-eabi -march=armv8.1-m.main+mve %s 2>%t
+# RUN:  FileCheck --check-prefix=ERROR-V81M_MVE < %t %s
+# RUN: not %clang -c -target arm-none-none-eabi -march=armv8.1-m.main+mve+fp %s 2>%t
+# RUN:  FileCheck --check-prefix=ERROR-V81M_MVE_FP < %t %s
+# RUN: not %clang -c -target arm-none-none-eabi -march=armv8.1-m.main+mve.fp %s 2>%t
+# RUN:  FileCheck --check-prefix=ERROR-V81M_MVEFP < %t %s
+# RUN: %clang -c -target arm-none-none-eabi -march=armv8.1-m.main+mve.fp+fp.dp %s
+
+.syntax unified
+.thumb
+.text
+
+csinc r0, r1, r2, eq
+# ERROR-V8M: :[[@LINE-1]]:1: error
+
+qadd r0, r1, r2
+# ERROR-V8M: :[[@LINE-1]]:1: error
+# ERROR-V81M: :[[@LINE-2]]:1: error
+# ERROR-V81M_FP: :[[@LINE-3]]:1: error
+# ERROR-V81M_FPDP: :[[@LINE-4]]:1: error
+
+vadd.f16 s0, s1, s2
+# ERROR-V8M: :[[@LINE-1]]:1: error
+# ERROR-V81M: :[[@LINE-2]]:1: error
+# ERROR-V81M_DSP: :[[@LINE-3]]:1: error
+# ERROR-V81M_MVE: :[[@LINE-4]]:1: error
+
+vabs.f32 s0, s1
+# ERROR-V8M: :[[@LINE-1]]:1: error
+# ERROR-V81M: :[[@LINE-2]]:1: error
+# ERROR-V81M_DSP: :[[@LINE-3]]:1: error
+# ERROR-V81M_MVE: :[[@LINE-4]]:1: error
+
+vcmp.f64 d0,d1
+# ERROR-V8M: :[[@LINE-1]]:1: error
+# ERROR-V81M: :[[@LINE-2]]:1: error
+# ERROR-V81M_DSP: :[[@LINE-3]]:1: error
+# ERROR-V81M_FP: :[[@LINE-4]]:1: error
+# ERROR-V81M_MVE: :[[@LINE-5]]:1: error
+# ERROR-V81M_MVE_FP: :[[@LINE-6]]:1: error
+# ERROR-V81M_MVEFP: :[[@LINE-7]]:1: error
+
+asrl r0, r1, r2
+# ERROR-V8M: :[[@LINE-1]]:1: error
+# ERROR-V81M: :[[@LINE-2]]:1: error
+# ERROR-V81M_DSP: :[[@LINE-3]]:1: error
+# ERROR-V81M_FP: :[[@LINE-4]]:1: error
+# ERROR-V81M_FPDP: :[[@LINE-5]]:1: error
+
+vcadd.i8 q0, q1, q2, #90
+# ERROR-V8M: :[[@LINE-1]]:1: error
+# ERROR-V81M: :[[@LINE-2]]:1: error
+# ERROR-V81M_DSP: :[[@LINE-3]]:1: error
+# ERROR-V81M_FP: :[[@LINE-4]]:1: error
+# ERROR-V81M_FPDP: :[[@LINE-5]]:1: error
Index: cfe/trunk/test/Driver/armv8.1m.main.c
===
--- cfe/trunk/test/Driver/armv8.1m.main.c
+++ cfe/trunk/test/Driver/armv8.1m.main.c
@@ -0,0 +1,34 @@
+// RUN: %clang -target arm-arm-none-eabi -march=armv8.1-m.main+dsp  -### %s 2> %t
+// RUN: FileCheck --check-prefix=CHECK-DSP < %t %s
+// CHECK-DSP: "-target-feature" "+dsp"
+
+// RUN: %clang -target arm-arm-none-eabi -march=armv8.1-m.main+fp  -### %s 2> %t
+// RUN: FileCheck --check-prefix=CHECK-FP < %t %s
+// CHECK-FP: "-target-feature" "+fp-armv8"
+// CHECK-FP-NOT: "-target-feature" "+fp64"
+// CHECK-FP-NOT: "-target-feature" "+d32"
+// CHECK-FP: "-target-feature" "+fullfp16"
+
+// RUN: %clang -target arm-arm-none-eabi -march=armv8.1-m.main+fp.dp  -### %s 2> %t
+// RUN: FileCheck --check-prefix=CHECK-FPDP < %t %s
+// CHECK-FPDP: "-target-feature" "+fp-armv8"
+// CHECK-FPDP: "-target-feature" "+fullfp16"
+// CHECK-FPDP: "-target-feature" "+fp64"
+// CHECK-FPDP-NOT: "-target-feature" "+d32"
+
+// RUN: %clang -target arm-arm-none-eabi -march=armv8.1-m.main+mve  -### %s 2> %t
+// RUN: FileCheck --check-prefix=CHECK-MVE < %t %s
+// CHECK-MVE: "-target-feature" "+mve"
+
+// RUN: %clang -target 

[PATCH] D60691: [ARM] Replace fp-only-sp and d16 with fp64 and d32.

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer accepted this revision.
SjoerdMeijer added a comment.
This revision is now accepted and ready to land.

There already was consensus that this is was a good change, and also that we 
don't care about auto-upgrading. With the last minor comments addressed, this 
looks good I think. Perhaps you can wait with committing until tomorrow morning 
just in case there are more comments.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60691/new/

https://reviews.llvm.org/D60691



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61717: Fix arm_neon.h to be clean under -fno-lax-vector-conversions.

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer accepted this revision.
SjoerdMeijer added a comment.
This revision is now accepted and ready to land.

Looks okay to me.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61717/new/

https://reviews.llvm.org/D61717



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D60699: [ARM] add CLI support for 8.1-M and MVE.

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer added inline comments.



Comment at: clang/test/Driver/armv8.1m.main.c:1
+// RUN: %clang -target arm-arm-none-eabi -march=armv8.1-m.main+dsp  -### %s 2> 
%t
+// RUN: FileCheck --check-prefix=CHECK-DSP < %t %s

It doesn't really matter, I guess, but we don't need a temp file and can pipe 
the output directly to FileCheck?



Comment at: clang/test/Driver/armv8.1m.main.c:3
+// RUN: FileCheck --check-prefix=CHECK-DSP < %t %s
+// CHECK-DSP: "-target-feature" "+dsp"
+

Do we also want to check that just:

  -march=armv8.1-m

doesn't enable DSP (and other non-mandatory extensions)?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60699/new/

https://reviews.llvm.org/D60699



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D53633: [AArch64] Implement FP16FML intrinsics

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer added a comment.

FYI: a new ACLE version has been published, please find it here:   
https://developer.arm.com/architectures/system-architectures/software-standards/acle

The "Neon Intrinsics" section contains these new intrinsics.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D53633/new/

https://reviews.llvm.org/D53633



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D58306: [AArch64] Change size suffix for FP16FML intrinsics.

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer accepted this revision.
SjoerdMeijer added a comment.
This revision is now accepted and ready to land.

LGTM

The ACLE has been updated and a new version with change included will be 
released soon.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58306/new/

https://reviews.llvm.org/D58306



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D58306: [AArch64] Change size suffix for FP16FML intrinsics.

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer added a comment.

I am discussing this with our GCC team as we would like both Clang/GCC 
implementation to be the same. But you're right that _f16 looks like to be the 
more consistent choice. I will let you know as soon I know more.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58306/new/

https://reviews.llvm.org/D58306



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D53633: [AArch64] Implement FP16FML intrinsics

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer added inline comments.



Comment at: cfe/trunk/test/CodeGen/aarch64-neon-fp16fml.c:12
+
+float32x2_t test_vfmlal_low_u32(float32x2_t a, float16x4_t b, float16x4_t c) {
+// CHECK-LABEL: define <2 x float> @test_vfmlal_low_u32(<2 x float> %a, <4 x 
half> %b, <4 x half> %c)

SjoerdMeijer wrote:
> ab wrote:
> > Hey folks, I'm curious: where does the "_u32" suffix come from? Should it 
> > be _f16?
> > 
> > Also, are there any new ACLE/intrinsic list documents? As far as I can tell 
> > there hasn't been any release since IHI0073B/IHI0053D.
> > Also, are there any new ACLE/intrinsic list documents? As far as I can tell 
> > there hasn't been any release since IHI0073B/IHI0053D.
> 
> I've checked, and an updated ACLE that includes these FP16FML intrinsics is 
> coming soon.
> 
> > where does the "_u32" suffix come from? Should it be _f16?
> 
> Good question. It could probably be _f32 or _f16, but _u32 doesn't seem to 
> make much sense. Looks like the spec says _u32, and that's also what GCC has 
> implemented. I think we want to update the spec and fix the name before the 
> updated spec is available. Will chase this, and let you know once I know more.
An update on this: we should change this to _f32 (because the first suffixes 
were refering to the ouput type). The ACLE will be updated accordingly, and 
also GCC will change its current implementation (from _u32 to _f32).  Many 
thanks for raising this issue.
Is there a volunteer to prepare a patch? Or do you have one already? :-) I 
could look at it, but that will be towards the end of next week.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D53633/new/

https://reviews.llvm.org/D53633



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D53633: [AArch64] Implement FP16FML intrinsics

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer added inline comments.



Comment at: cfe/trunk/test/CodeGen/aarch64-neon-fp16fml.c:12
+
+float32x2_t test_vfmlal_low_u32(float32x2_t a, float16x4_t b, float16x4_t c) {
+// CHECK-LABEL: define <2 x float> @test_vfmlal_low_u32(<2 x float> %a, <4 x 
half> %b, <4 x half> %c)

ab wrote:
> Hey folks, I'm curious: where does the "_u32" suffix come from? Should it be 
> _f16?
> 
> Also, are there any new ACLE/intrinsic list documents? As far as I can tell 
> there hasn't been any release since IHI0073B/IHI0053D.
> Also, are there any new ACLE/intrinsic list documents? As far as I can tell 
> there hasn't been any release since IHI0073B/IHI0053D.

I've checked, and an updated ACLE that includes these FP16FML intrinsics is 
coming soon.

> where does the "_u32" suffix come from? Should it be _f16?

Good question. It could probably be _f32 or _f16, but _u32 doesn't seem to make 
much sense. Looks like the spec says _u32, and that's also what GCC has 
implemented. I think we want to update the spec and fix the name before the 
updated spec is available. Will chase this, and let you know once I know more.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D53633/new/

https://reviews.llvm.org/D53633



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57577: Make predefined FLT16 macros conditional on support for the type

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer accepted this revision.
SjoerdMeijer added a comment.
This revision is now accepted and ready to land.

Looks okay to me, with one nit inline.




Comment at: test/Preprocessor/init.c:9169
 // WEBASSEMBLY-NEXT:#define __FLOAT128__ 1
-// WEBASSEMBLY-NEXT:#define __FLT16_DECIMAL_DIG__ 5
-// WEBASSEMBLY-NEXT:#define __FLT16_DENORM_MIN__ 5.9604644775390625e-8F16

Perhaps change this in WEBASSEMBLY-NOT so that we also have one negative test 
for this?


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57577/new/

https://reviews.llvm.org/D57577



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57188: Disable _Float16 for non ARM/SPIR Targets

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer added inline comments.



Comment at: include/clang/Basic/TargetInfo.h:66
   bool HasFloat128;
+  bool HasFloat16;
   unsigned char PointerWidth, PointerAlign;

I think this is the same as `HasLegalHalfType`, and we can (re)use that.  Or, 
at least, don't think we need both `HasLegalHalfType` and `HasFloat16`. For 
context, I needed `HasLegalHalfType` for argument passing, but it looks like it 
can serve another purpose now.

Out of curiousity, I was wondering if specifying:

  KEYWORD(_Float16, HALFSUPPORT)

in TokenKids.def is an alternative approach (it is currently set to KEYALL). 
Thus, enable the keyword when `LangOpts.Half` is set. By adding this 
`HasFloat16` property here in clang's targetinfo, we're sort of defining again 
how targets support different types. I.e., if you throw a `half` type at the 
backend, the TypeLegalizer will deal with it in one way or another. Perhaps 
disabling `_Float16` can be achieved by disabling the keyword. But I do see 
that the big advantage of this patch is the much nicer error message (otherwise 
we would get something like  "unknown type name '_Float16'").




Comment at: include/clang/Basic/TargetInfo.h:521
+  /// Determine whether the _Float16 type is supported on this target.
+  virtual bool hasFloat16Type() const { return HasFloat16; }
+

Similar remark: the same as `hasLegalHalfType()`?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57188/new/

https://reviews.llvm.org/D57188



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50179: [AArch64][ARM] Context sensitive meaning of option "crypto"

[Sjoerd Meijer via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL343758: [AArch64][ARM] Context sensitive meaning of crypto 
(authored by SjoerdMeijer, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D50179?vs=166643=168233#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D50179

Files:
  cfe/trunk/lib/Driver/ToolChains/Arch/AArch64.cpp
  cfe/trunk/lib/Driver/ToolChains/Arch/ARM.cpp
  cfe/trunk/test/Driver/arm-features.c
  cfe/trunk/test/Preprocessor/aarch64-target-features.c

Index: cfe/trunk/lib/Driver/ToolChains/Arch/AArch64.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/Arch/AArch64.cpp
+++ cfe/trunk/lib/Driver/ToolChains/Arch/AArch64.cpp
@@ -219,6 +219,87 @@
   Features.push_back("+fullfp16");
   }
 
+  // FIXME: this needs reimplementation too after the TargetParser rewrite
+  //
+  // Context sensitive meaning of Crypto:
+  // 1) For Arch >= ARMv8.4a:  crypto = sm4 + sha3 + sha2 + aes
+  // 2) For Arch <= ARMv8.3a:  crypto = sha2 + aes
+  const auto ItBegin = Features.begin();
+  const auto ItEnd = Features.end();
+  const auto ItRBegin = Features.rbegin();
+  const auto ItREnd = Features.rend();
+  const auto ItRCrypto = std::find(ItRBegin, ItREnd, "+crypto");
+  const auto ItRNoCrypto = std::find(ItRBegin, ItREnd, "-crypto");
+  const auto HasCrypto  = ItRCrypto != ItREnd;
+  const auto HasNoCrypto = ItRNoCrypto != ItREnd;
+  const ptrdiff_t PosCrypto = ItRCrypto - ItRBegin;
+  const ptrdiff_t PosNoCrypto = ItRNoCrypto - ItRBegin;
+
+  bool NoCrypto = false;
+  if (HasCrypto && HasNoCrypto) {
+if (PosNoCrypto < PosCrypto)
+  NoCrypto = true;
+  }
+
+  if (std::find(ItBegin, ItEnd, "+v8.4a") != ItEnd) {
+if (HasCrypto && !NoCrypto) {
+  // Check if we have NOT disabled an algorithm with something like:
+  //   +crypto, -algorithm
+  // And if "-algorithm" does not occur, we enable that crypto algorithm.
+  const bool HasSM4  = (std::find(ItBegin, ItEnd, "-sm4") == ItEnd);
+  const bool HasSHA3 = (std::find(ItBegin, ItEnd, "-sha3") == ItEnd);
+  const bool HasSHA2 = (std::find(ItBegin, ItEnd, "-sha2") == ItEnd);
+  const bool HasAES  = (std::find(ItBegin, ItEnd, "-aes") == ItEnd);
+  if (HasSM4)
+Features.push_back("+sm4");
+  if (HasSHA3)
+Features.push_back("+sha3");
+  if (HasSHA2)
+Features.push_back("+sha2");
+  if (HasAES)
+Features.push_back("+aes");
+} else if (HasNoCrypto) {
+  // Check if we have NOT enabled a crypto algorithm with something like:
+  //   -crypto, +algorithm
+  // And if "+algorithm" does not occur, we disable that crypto algorithm.
+  const bool HasSM4  = (std::find(ItBegin, ItEnd, "+sm4") != ItEnd);
+  const bool HasSHA3 = (std::find(ItBegin, ItEnd, "+sha3") != ItEnd);
+  const bool HasSHA2 = (std::find(ItBegin, ItEnd, "+sha2") != ItEnd);
+  const bool HasAES  = (std::find(ItBegin, ItEnd, "+aes") != ItEnd);
+  if (!HasSM4)
+Features.push_back("-sm4");
+  if (!HasSHA3)
+Features.push_back("-sha3");
+  if (!HasSHA2)
+Features.push_back("-sha2");
+  if (!HasAES)
+Features.push_back("-aes");
+}
+  } else {
+if (HasCrypto && !NoCrypto) {
+  const bool HasSHA2 = (std::find(ItBegin, ItEnd, "-sha2") == ItEnd);
+  const bool HasAES = (std::find(ItBegin, ItEnd, "-aes") == ItEnd);
+  if (HasSHA2)
+Features.push_back("+sha2");
+  if (HasAES)
+Features.push_back("+aes");
+} else if (HasNoCrypto) {
+  const bool HasSHA2 = (std::find(ItBegin, ItEnd, "+sha2") != ItEnd);
+  const bool HasAES  = (std::find(ItBegin, ItEnd, "+aes") != ItEnd);
+  const bool HasV82a = (std::find(ItBegin, ItEnd, "+v8.2a") != ItEnd);
+  const bool HasV83a = (std::find(ItBegin, ItEnd, "+v8.3a") != ItEnd);
+  const bool HasV84a = (std::find(ItBegin, ItEnd, "+v8.4a") != ItEnd);
+  if (!HasSHA2)
+Features.push_back("-sha2");
+  if (!HasAES)
+Features.push_back("-aes");
+  if (HasV82a || HasV83a || HasV84a) {
+Features.push_back("-sm4");
+Features.push_back("-sha3");
+  }
+}
+  }
+
   if (Arg *A = Args.getLastArg(options::OPT_mno_unaligned_access,
options::OPT_munaligned_access))
 if (A->getOption().matches(options::OPT_mno_unaligned_access))
Index: cfe/trunk/lib/Driver/ToolChains/Arch/ARM.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/Arch/ARM.cpp
+++ cfe/trunk/lib/Driver/ToolChains/Arch/ARM.cpp
@@ -444,6 +444,26 @@
   Features.push_back("-crc");
   }
 
+  // For Arch >= ARMv8.0:  crypto = sha2 + aes
+  // FIXME: this needs reimplementation after the TargetParser rewrite
+  if (ArchName.find_lower("armv8a") != StringRef::npos ||
+  ArchName.find_lower("armv8.1a") != 

[PATCH] D50179: [AArch64][ARM] Context sensitive meaning of option "crypto"

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer added a comment.

Thanks!


https://reviews.llvm.org/D50179



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50179: [AArch64][ARM] Context sensitive meaning of option "crypto"

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer added a comment.

@efriedma : apologies for the ping, but does this look reasonable?


https://reviews.llvm.org/D50179



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52493: [AArch64][v8.5A] Test clang option for the Memory Tagging Extension

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer accepted this revision.
SjoerdMeijer added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rC Clang

https://reviews.llvm.org/D52493



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52492: [AArch64][v8.5A] Test optional Armv8.5-A random number extension

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer accepted this revision.
SjoerdMeijer added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rC Clang

https://reviews.llvm.org/D52492



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52491: [ARM/AArch64][v8.5A] Add Armv8.5-A target

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer accepted this revision.
SjoerdMeijer added a comment.
This revision is now accepted and ready to land.

Looks okay to me




Comment at: test/Driver/arm-cortex-cpus.c:338
+
+// RUN: %clang -target armv8a-linux-eabi -march=armv8.5-a+fp16 -### -c %s 2>&1 
| FileCheck --check-prefix CHECK-V85A-FP16 %s
+// CHECK-V85A-FP16: "-cc1"{{.*}} "-triple" "armv8.5{{.*}}" "-target-cpu" 
"generic" {{.*}}"-target-feature" "+fullfp16"

nit: perhaps move this to below, where we have the other fp16 checks?



Comment at: test/Preprocessor/arm-target-features.c:746
+
+// RUN: %clang -target armv8.4a-none-none-eabi -x c -E -dM %s -o - | FileCheck 
-match-full-lines --check-prefix=CHECK-V84A %s
+// CHECK-V84A: #define __ARM_ARCH 8

thanks for upstreaming a little bit of v8.3 and v8.4 too :-)


Repository:
  rC Clang

https://reviews.llvm.org/D52491



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50229: [ARM][AArch64] Add feature +fp16fml

[Sjoerd Meijer via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC342862: [ARM][AArch64] Add feature +fp16fml (authored by 
SjoerdMeijer, committed by ).

Repository:
  rC Clang

https://reviews.llvm.org/D50229

Files:
  lib/Driver/ToolChains/Arch/AArch64.cpp
  lib/Driver/ToolChains/Arch/ARM.cpp
  test/Driver/aarch64-cpus.c
  test/Driver/arm-cortex-cpus.c
  test/Preprocessor/aarch64-target-features.c
  test/Preprocessor/arm-target-features.c

Index: lib/Driver/ToolChains/Arch/AArch64.cpp
===
--- lib/Driver/ToolChains/Arch/AArch64.cpp
+++ lib/Driver/ToolChains/Arch/AArch64.cpp
@@ -193,6 +193,32 @@
   Features.push_back("-crc");
   }
 
+  // Handle (arch-dependent) fp16fml/fullfp16 relationship.
+  // FIXME: this fp16fml option handling will be reimplemented after the
+  // TargetParser rewrite.
+  const auto ItRNoFullFP16 = std::find(Features.rbegin(), Features.rend(), "-fullfp16");
+  const auto ItRFP16FML = std::find(Features.rbegin(), Features.rend(), "+fp16fml");
+  if (std::find(Features.begin(), Features.end(), "+v8.4a") != Features.end()) {
+const auto ItRFullFP16  = std::find(Features.rbegin(), Features.rend(), "+fullfp16");
+if (ItRFullFP16 < ItRNoFullFP16 && ItRFullFP16 < ItRFP16FML) {
+  // Only entangled feature that can be to the right of this +fullfp16 is -fp16fml.
+  // Only append the +fp16fml if there is no -fp16fml after the +fullfp16.
+  if (std::find(Features.rbegin(), ItRFullFP16, "-fp16fml") == ItRFullFP16)
+Features.push_back("+fp16fml");
+}
+else
+  goto fp16_fml_fallthrough;
+  }
+  else {
+fp16_fml_fallthrough:
+// In both of these cases, putting the 'other' feature on the end of the vector will
+// result in the same effect as placing it immediately after the current feature.
+if (ItRNoFullFP16 < ItRFP16FML)
+  Features.push_back("-fp16fml");
+else if (ItRNoFullFP16 > ItRFP16FML)
+  Features.push_back("+fullfp16");
+  }
+
   if (Arg *A = Args.getLastArg(options::OPT_mno_unaligned_access,
options::OPT_munaligned_access))
 if (A->getOption().matches(options::OPT_mno_unaligned_access))
Index: lib/Driver/ToolChains/Arch/ARM.cpp
===
--- lib/Driver/ToolChains/Arch/ARM.cpp
+++ lib/Driver/ToolChains/Arch/ARM.cpp
@@ -391,6 +391,33 @@
   } else if (HDivArg)
 getARMHWDivFeatures(D, HDivArg, Args, HDivArg->getValue(), Features);
 
+  // Handle (arch-dependent) fp16fml/fullfp16 relationship.
+  // Must happen before any features are disabled due to soft-float.
+  // FIXME: this fp16fml option handling will be reimplemented after the
+  // TargetParser rewrite.
+  const auto ItRNoFullFP16 = std::find(Features.rbegin(), Features.rend(), "-fullfp16");
+  const auto ItRFP16FML = std::find(Features.rbegin(), Features.rend(), "+fp16fml");
+  if (Triple.getSubArch() == llvm::Triple::SubArchType::ARMSubArch_v8_4a) {
+const auto ItRFullFP16  = std::find(Features.rbegin(), Features.rend(), "+fullfp16");
+if (ItRFullFP16 < ItRNoFullFP16 && ItRFullFP16 < ItRFP16FML) {
+  // Only entangled feature that can be to the right of this +fullfp16 is -fp16fml.
+  // Only append the +fp16fml if there is no -fp16fml after the +fullfp16.
+  if (std::find(Features.rbegin(), ItRFullFP16, "-fp16fml") == ItRFullFP16)
+Features.push_back("+fp16fml");
+}
+else
+  goto fp16_fml_fallthrough;
+  }
+  else {
+fp16_fml_fallthrough:
+// In both of these cases, putting the 'other' feature on the end of the vector will
+// result in the same effect as placing it immediately after the current feature.
+if (ItRNoFullFP16 < ItRFP16FML)
+  Features.push_back("-fp16fml");
+else if (ItRNoFullFP16 > ItRFP16FML)
+  Features.push_back("+fullfp16");
+  }
+
   // Setting -msoft-float/-mfloat-abi=soft effectively disables the FPU (GCC
   // ignores the -mfpu options in this case).
   // Note that the ABI can also be set implicitly by the target selected.
@@ -404,7 +431,7 @@
 //now just be explicit and disable all known dependent features
 //as well.
 for (std::string Feature : {"vfp2", "vfp3", "vfp4", "fp-armv8", "fullfp16",
-"neon", "crypto", "dotprod"})
+"neon", "crypto", "dotprod", "fp16fml"})
   if (std::find(std::begin(Features), std::end(Features), "+" + Feature) != std::end(Features))
 Features.push_back(Args.MakeArgString("-" + Feature));
   }
Index: test/Preprocessor/aarch64-target-features.c
===
--- test/Preprocessor/aarch64-target-features.c
+++ test/Preprocessor/aarch64-target-features.c
@@ -93,18 +93,45 @@
 // RUN: %clang -target aarch64-none-linux-gnu -march=armv8.2a+dotprod -x c -E -dM %s -o - | FileCheck --check-prefix=CHECK-DOTPROD %s
 // 

[PATCH] D50179: [AArch64][ARM] Context sensitive meaning of option "crypto"

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer updated this revision to Diff 166643.
SjoerdMeijer added a comment.

Added FIXMEs, like in https://reviews.llvm.org/D50229, that this needs 
reimplementation too after the TargerParser rewrite.

About v8.5, the ISA description is now available here: 
https://developer.arm.com/products/architecture/cpu-architecture/a-profile/exploration-tools

But we will add support for that when we upstream v8.5 support, so will be 
added later.


https://reviews.llvm.org/D50179

Files:
  lib/Driver/ToolChains/Arch/AArch64.cpp
  lib/Driver/ToolChains/Arch/ARM.cpp
  test/Driver/arm-features.c
  test/Preprocessor/aarch64-target-features.c

Index: test/Preprocessor/aarch64-target-features.c
===
--- test/Preprocessor/aarch64-target-features.c
+++ test/Preprocessor/aarch64-target-features.c
@@ -170,6 +170,101 @@
 // CHECK-MARCH-2: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-feature" "-fp-armv8" "-target-feature" "-neon" "-target-feature" "-crc" "-target-feature" "-crypto"
 // CHECK-MARCH-3: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-feature" "-neon"
 
+// Check +sm4:
+//
+// RUN: %clang -target aarch64 -march=armv8.2a+sm4 -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-SM4 %s
+// CHECK-SM4:  "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-feature" "+v8.2a" "-target-feature" "+sm4"
+//
+// Check +sha3:
+//
+// RUN: %clang -target aarch64 -march=armv8.2a+sha3 -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-SHA3 %s
+// CHECK-SHA3: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-feature" "+v8.2a" "-target-feature" "+sha3"
+//
+// Check +sha2:
+//
+// RUN: %clang -target aarch64 -march=armv8.3a+sha2 -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-SHA2 %s
+// CHECK-SHA2: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-feature" "+v8.{{.}}a" "-target-feature" "+sha2"
+//
+// Check +aes:
+//
+// RUN: %clang -target aarch64 -march=armv8.3a+aes -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-AES %s
+// CHECK-AES: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-feature" "+v8.{{.}}a" "-target-feature" "+aes"
+//
+// Check -sm4:
+//
+// RUN: %clang -target aarch64 -march=armv8.2a+noSM4 -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-NO-SM4 %s
+// CHECK-NO-SM4:  "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-feature" "+v8.2a" "-target-feature" "-sm4"
+//
+// Check -sha3:
+//
+// RUN: %clang -target aarch64 -march=armv8.2a+noSHA3 -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-NO-SHA3 %s
+// CHECK-NO-SHA3: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-feature" "+v8.2a" "-target-feature" "-sha3"
+//
+// Check -sha2:
+//
+// RUN: %clang -target aarch64 -march=armv8.2a+noSHA2 -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-NO-SHA2 %s
+// CHECK-NO-SHA2: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-feature" "+v8.2a" "-target-feature" "-sha2"
+//
+// Check -aes:
+//
+// RUN: %clang -target aarch64 -march=armv8.2a+noAES -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-NO-AES %s
+// CHECK-NO-AES: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-feature" "+v8.2a" "-target-feature" "-aes"
+//
+//
+// Arch <= ARMv8.3:  crypto = sha2 + aes
+// -
+//
+// Check +crypto:
+//
+// RUN: %clang -target aarch64 -march=armv8a+crypto -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-CRYPTO83 %s
+// RUN: %clang -target aarch64 -march=armv8.1a+crypto -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-CRYPTO83 %s
+// RUN: %clang -target aarch64 -march=armv8.2a+crypto -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-CRYPTO83 %s
+// RUN: %clang -target aarch64 -march=armv8.3a+crypto -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-CRYPTO83 %s
+// RUN: %clang -target aarch64 -march=armv8a+crypto+nocrypto+crypto -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-CRYPTO83 %s
+// CHECK-CRYPTO83: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-feature" "+crypto" "-target-feature" "+sha2" "-target-feature" "+aes"
+//
+// Check -crypto:
+//
+// RUN: %clang -target aarch64 -march=armv8a+nocrypto -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-NOCRYPTO8A %s
+// RUN: %clang -target aarch64 -march=armv8.1a+nocrypto -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-NOCRYPTO81 %s
+// RUN: %clang -target aarch64 -march=armv8.2a+nocrypto -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-NOCRYPTO82 %s
+// RUN: %clang -target aarch64 -march=armv8.3a+nocrypto -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-NOCRYPTO82 %s
+// RUN: %clang -target aarch64 -march=armv8.3a+nocrypto+crypto+nocrypto -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-NOCRYPTO82 %s
+
+// CHECK-NOCRYPTO8A: "-target-feature" "+neon" "-target-feature" "-crypto" "-target-feature" "-sha2" "-target-feature" "-aes" "-target-abi" "aapcs"
+// CHECK-NOCRYPTO81: "-target-feature" "+neon" "-target-feature" "+v8.1a" "-target-feature" "-crypto" "-target-feature" "-sha2" "-target-feature" "-aes" "-target-abi" "aapcs"
+// CHECK-NOCRYPTO82: "-target-feature" "+neon" "-target-feature" "+v8.{{.}}a" "-target-feature" 

[PATCH] D50229: [ARM][AArch64] Add feature +fp16fml

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer updated this revision to Diff 166439.
SjoerdMeijer retitled this revision from "+fp16fml feature for ARM and AArch64" 
to "[ARM][AArch64] Add feature +fp16fml".
SjoerdMeijer edited the summary of this revision.
SjoerdMeijer added a comment.

Added FIXMEs.


https://reviews.llvm.org/D50229

Files:
  lib/Driver/ToolChains/Arch/AArch64.cpp
  lib/Driver/ToolChains/Arch/ARM.cpp
  test/Driver/aarch64-cpus.c
  test/Driver/arm-cortex-cpus.c
  test/Preprocessor/aarch64-target-features.c
  test/Preprocessor/arm-target-features.c

Index: test/Preprocessor/arm-target-features.c
===
--- test/Preprocessor/arm-target-features.c
+++ test/Preprocessor/arm-target-features.c
@@ -21,18 +21,58 @@
 // CHECK-V8A-ALLOW-FP-INSTR: #define __ARM_FP16_FORMAT_IEEE 1
 // CHECK-V8A-ALLOW-FP-INSTR-V8A-NOT: #define __ARM_FEATURE_DOTPROD
 
-// RUN: %clang -target arm-none-linux-gnueabi -march=armv8.2a+fp16 -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-FULLFP16-VECTOR-SCALAR %s
+// RUN: %clang -target arm-none-linux-gnueabi -march=armv8.2-a+nofp16fml+fp16 -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-FULLFP16-VECTOR-SCALAR %s
+// RUN: %clang -target arm-none-linux-gnueabi -march=armv8.2-a+nofp16+fp16fml -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-FULLFP16-VECTOR-SCALAR %s
+// RUN: %clang -target arm-none-linux-gnueabi -march=armv8.2-a+fp16+nofp16fml -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-FULLFP16-VECTOR-SCALAR %s
+// RUN: %clang -target arm-none-linux-gnueabi -march=armv8-a+fp16fml -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-FULLFP16-VECTOR-SCALAR %s
+// RUN: %clang -target arm-none-linux-gnueabi -march=armv8-a+fp16 -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-FULLFP16-VECTOR-SCALAR %s
+// RUN: %clang -target arm-none-linux-gnueabi -march=armv8.4-a+nofp16fml+fp16 -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-FULLFP16-VECTOR-SCALAR %s
+// RUN: %clang -target arm-none-linux-gnueabi -march=armv8.4-a+nofp16+fp16fml -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-FULLFP16-VECTOR-SCALAR %s
+// RUN: %clang -target arm-none-linux-gnueabi -march=armv8.4-a+fp16+nofp16fml -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-FULLFP16-VECTOR-SCALAR %s
+// RUN: %clang -target arm-none-linux-gnueabi -march=armv8.4-a+fp16fml -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-FULLFP16-VECTOR-SCALAR %s
+// RUN: %clang -target arm-none-linux-gnueabi -march=armv8.4-a+fp16 -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-FULLFP16-VECTOR-SCALAR %s
 // CHECK-FULLFP16-VECTOR-SCALAR: #define __ARM_FEATURE_FP16_SCALAR_ARITHMETIC 1
 // CHECK-FULLFP16-VECTOR-SCALAR: #define __ARM_FEATURE_FP16_VECTOR_ARITHMETIC 1
 // CHECK-FULLFP16-VECTOR-SCALAR: #define __ARM_FP 0xe
 // CHECK-FULLFP16-VECTOR-SCALAR: #define __ARM_FP16_FORMAT_IEEE 1
 
-// RUN: %clang -target arm-none-linux-gnueabi -march=armv8.2a+fp16 -mfpu=vfp4 -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-FULLFP16-SCALAR %s
+// +fp16fml without neon doesn't make sense as the fp16fml instructions all require SIMD.
+// However, as +fp16fml implies +fp16 there is a set of defines that we would expect.
+// RUN: %clang -target arm-none-linux-gnueabi -march=armv8-a+fp16fml -mfpu=vfp4 -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-FULLFP16-SCALAR %s
+// RUN: %clang -target arm-none-linux-gnueabi -march=armv8-a+fp16 -mfpu=vfp4 -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-FULLFP16-SCALAR %s
+// RUN: %clang -target arm-none-linux-gnueabi -march=armv8.4-a+fp16fml -mfpu=vfp4 -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-FULLFP16-SCALAR %s
+// RUN: %clang -target arm-none-linux-gnueabi -march=armv8.4-a+fp16 -mfpu=vfp4 -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-FULLFP16-SCALAR %s
 // CHECK-FULLFP16-SCALAR:   #define __ARM_FEATURE_FP16_SCALAR_ARITHMETIC 1
 // CHECK-FULLFP16-SCALAR-NOT:   #define __ARM_FEATURE_FP16_VECTOR_ARITHMETIC 1
 // CHECK-FULLFP16-SCALAR:   #define __ARM_FP 0xe
 // CHECK-FULLFP16-SCALAR:   #define __ARM_FP16_FORMAT_IEEE 1
-//
+
+// RUN: %clang -target arm-none-linux-gnueabi -march=armv8.2-a -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-FULLFP16-NOFML-VECTOR-SCALAR %s
+// RUN: %clang -target arm-none-linux-gnueabi -march=armv8.2-a+nofp16 -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-FULLFP16-NOFML-VECTOR-SCALAR %s
+// RUN: %clang -target arm-none-linux-gnueabi -march=armv8.2-a+nofp16fml -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-FULLFP16-NOFML-VECTOR-SCALAR %s
+// RUN: %clang -target arm-none-linux-gnueabi -march=armv8.2-a+fp16fml+nofp16 -x c -E -dM %s -o 

[PATCH] D50229: +fp16fml feature for ARM and AArch64

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer added a comment.

Ah, and just for your info, the proposal was just sent to the dev list: 
http://lists.llvm.org/pipermail/llvm-dev/2018-September/126346.html


Repository:
  rC Clang

https://reviews.llvm.org/D50229



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50229: +fp16fml feature for ARM and AArch64

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer added a comment.

(I am now picking this up, and will try to progress this patch and also 
https://reviews.llvm.org/D50179)

> Do you expect that the regression tests will be affected by the TargetParser 
> fixes?

No, and that's exactly the reason why it would be nice to get this in. The 
tests won't change, they show the expected behaviour, and thus we have a sort 
of "baseline implementation" while we are working on the new options framework.

And just repeating what I said in the other ticket, this option handling 
implementation is far from ideal and pretty, it's very easy to agree on that. 
This is a low maintenance patch, so very easy to keep downstream for us, but it 
would be useful to have it on trunk too perhaps.

I will add comments and a FIXME that we expect a full reimplementation of it.


Repository:
  rC Clang

https://reviews.llvm.org/D50229



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51429: [AArch64] Return Address Signing B Key Support

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer added inline comments.



Comment at: test/CodeGen/aarch64-sign-return-address.c:3
 // RUN: %clang -target aarch64-arm-none-eabi -S -emit-llvm -o - 
-msign-return-address=non-leaf %s | FileCheck %s --check-prefix=CHECK-PARTIAL
 // RUN: %clang -target aarch64-arm-none-eabi -S -emit-llvm -o - 
-msign-return-address=all %s | FileCheck %s --check-prefix=CHECK-ALL
+// RUN: %clang -target aarch64-arm-none-eabi -S -emit-llvm -o - 
-msign-return-address=all+a_key %s | FileCheck %s --check-prefix=CHECK-A-KEY

If the default is the a_key, does this test need to check the a_key attribute?


Repository:
  rC Clang

https://reviews.llvm.org/D51429



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51093: [ARM] Set __ARM_FEATURE_SIMD32 for +dsp cores

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer accepted this revision.
SjoerdMeijer added a comment.
This revision is now accepted and ready to land.

Looks reasonable to me.


https://reviews.llvm.org/D51093



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50179: [AArch64][ARM] Context sensitive meaning of option "crypto"

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer updated this revision to Diff 159991.

https://reviews.llvm.org/D50179

Files:
  lib/Driver/ToolChains/Arch/AArch64.cpp
  lib/Driver/ToolChains/Arch/ARM.cpp
  test/Driver/arm-features.c
  test/Preprocessor/aarch64-target-features.c

Index: test/Preprocessor/aarch64-target-features.c
===
--- test/Preprocessor/aarch64-target-features.c
+++ test/Preprocessor/aarch64-target-features.c
@@ -143,6 +143,101 @@
 // CHECK-MARCH-2: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-feature" "-fp-armv8" "-target-feature" "-neon" "-target-feature" "-crc" "-target-feature" "-crypto"
 // CHECK-MARCH-3: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-feature" "-neon"
 
+// Check +sm4:
+//
+// RUN: %clang -target aarch64 -march=armv8.2a+sm4 -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-SM4 %s
+// CHECK-SM4:  "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-feature" "+v8.2a" "-target-feature" "+sm4"
+//
+// Check +sha3:
+//
+// RUN: %clang -target aarch64 -march=armv8.2a+sha3 -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-SHA3 %s
+// CHECK-SHA3: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-feature" "+v8.2a" "-target-feature" "+sha3"
+//
+// Check +sha2:
+//
+// RUN: %clang -target aarch64 -march=armv8.3a+sha2 -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-SHA2 %s
+// CHECK-SHA2: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-feature" "+v8.{{.}}a" "-target-feature" "+sha2"
+//
+// Check +aes:
+//
+// RUN: %clang -target aarch64 -march=armv8.3a+aes -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-AES %s
+// CHECK-AES: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-feature" "+v8.{{.}}a" "-target-feature" "+aes"
+//
+// Check -sm4:
+//
+// RUN: %clang -target aarch64 -march=armv8.2a+noSM4 -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-NO-SM4 %s
+// CHECK-NO-SM4:  "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-feature" "+v8.2a" "-target-feature" "-sm4"
+//
+// Check -sha3:
+//
+// RUN: %clang -target aarch64 -march=armv8.2a+noSHA3 -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-NO-SHA3 %s
+// CHECK-NO-SHA3: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-feature" "+v8.2a" "-target-feature" "-sha3"
+//
+// Check -sha2:
+//
+// RUN: %clang -target aarch64 -march=armv8.2a+noSHA2 -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-NO-SHA2 %s
+// CHECK-NO-SHA2: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-feature" "+v8.2a" "-target-feature" "-sha2"
+//
+// Check -aes:
+//
+// RUN: %clang -target aarch64 -march=armv8.2a+noAES -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-NO-AES %s
+// CHECK-NO-AES: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-feature" "+v8.2a" "-target-feature" "-aes"
+//
+//
+// Arch <= ARMv8.3:  crypto = sha2 + aes
+// -
+//
+// Check +crypto:
+//
+// RUN: %clang -target aarch64 -march=armv8a+crypto -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-CRYPTO83 %s
+// RUN: %clang -target aarch64 -march=armv8.1a+crypto -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-CRYPTO83 %s
+// RUN: %clang -target aarch64 -march=armv8.2a+crypto -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-CRYPTO83 %s
+// RUN: %clang -target aarch64 -march=armv8.3a+crypto -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-CRYPTO83 %s
+// RUN: %clang -target aarch64 -march=armv8a+crypto+nocrypto+crypto -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-CRYPTO83 %s
+// CHECK-CRYPTO83: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-feature" "+crypto" "-target-feature" "+sha2" "-target-feature" "+aes"
+//
+// Check -crypto:
+//
+// RUN: %clang -target aarch64 -march=armv8a+nocrypto -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-NOCRYPTO8A %s
+// RUN: %clang -target aarch64 -march=armv8.1a+nocrypto -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-NOCRYPTO81 %s
+// RUN: %clang -target aarch64 -march=armv8.2a+nocrypto -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-NOCRYPTO82 %s
+// RUN: %clang -target aarch64 -march=armv8.3a+nocrypto -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-NOCRYPTO82 %s
+// RUN: %clang -target aarch64 -march=armv8.3a+nocrypto+crypto+nocrypto -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-NOCRYPTO82 %s
+
+// CHECK-NOCRYPTO8A: "-target-feature" "+neon" "-target-feature" "-crypto" "-target-feature" "-sha2" "-target-feature" "-aes" "-target-abi" "aapcs"
+// CHECK-NOCRYPTO81: "-target-feature" "+neon" "-target-feature" "+v8.1a" "-target-feature" "-crypto" "-target-feature" "-sha2" "-target-feature" "-aes" "-target-abi" "aapcs"
+// CHECK-NOCRYPTO82: "-target-feature" "+neon" "-target-feature" "+v8.{{.}}a" "-target-feature" "-crypto" "-target-feature" "-sha2" "-target-feature" "-aes" "-target-feature" "-sm4" "-target-feature" "-sha3" "-target-abi" "aapcs"
+//
+// Check +crypto -sha2 -aes:
+//
+// RUN: %clang -target aarch64 -march=armv8.1a+crypto+nosha2+noaes -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-CRYPTO83-NOSHA2-NOAES %s
+// CHECK-CRYPTO83-NOSHA2-NOAES-NOT: "-target-feature" "+sha2" "-target-feature" "+aes"
+//

[PATCH] D50179: [AArch64][ARM] Context sensitive meaning of option "crypto"

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer updated this revision to Diff 159979.
SjoerdMeijer added a comment.

fixed typo


https://reviews.llvm.org/D50179

Files:
  lib/Driver/ToolChains/Arch/AArch64.cpp
  lib/Driver/ToolChains/Arch/ARM.cpp
  test/Driver/arm-features.c
  test/Preprocessor/aarch64-target-features.c

Index: test/Preprocessor/aarch64-target-features.c
===
--- test/Preprocessor/aarch64-target-features.c
+++ test/Preprocessor/aarch64-target-features.c
@@ -143,6 +143,101 @@
 // CHECK-MARCH-2: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-feature" "-fp-armv8" "-target-feature" "-neon" "-target-feature" "-crc" "-target-feature" "-crypto"
 // CHECK-MARCH-3: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-feature" "-neon"
 
+// Check +sm4:
+//
+// RUN: %clang -target aarch64 -march=armv8.2a+sm4 -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-SM4 %s
+// CHECK-SM4:  "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-feature" "+v8.2a" "-target-feature" "+sm4"
+//
+// Check +sha3:
+//
+// RUN: %clang -target aarch64 -march=armv8.2a+sha3 -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-SHA3 %s
+// CHECK-SHA3: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-feature" "+v8.2a" "-target-feature" "+sha3"
+//
+// Check +sha2:
+//
+// RUN: %clang -target aarch64 -march=armv8.3a+sha2 -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-SHA2 %s
+// CHECK-SHA2: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-feature" "+v8.{{.}}a" "-target-feature" "+sha2"
+//
+// Check +aes:
+//
+// RUN: %clang -target aarch64 -march=armv8.3a+aes -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-AES %s
+// CHECK-AES: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-feature" "+v8.{{.}}a" "-target-feature" "+aes"
+//
+// Check -sm4:
+//
+// RUN: %clang -target aarch64 -march=armv8.2a+noSM4 -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-NO-SM4 %s
+// CHECK-NO-SM4:  "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-feature" "+v8.2a" "-target-feature" "-sm4"
+//
+// Check -sha3:
+//
+// RUN: %clang -target aarch64 -march=armv8.2a+noSHA3 -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-NO-SHA3 %s
+// CHECK-NO-SHA3: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-feature" "+v8.2a" "-target-feature" "-sha3"
+//
+// Check -sha2:
+//
+// RUN: %clang -target aarch64 -march=armv8.2a+noSHA2 -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-NO-SHA2 %s
+// CHECK-NO-SHA2: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-feature" "+v8.2a" "-target-feature" "-sha2"
+//
+// Check -aes:
+//
+// RUN: %clang -target aarch64 -march=armv8.2a+noAES -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-NO-AES %s
+// CHECK-NO-AES: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-feature" "+v8.2a" "-target-feature" "-aes"
+//
+//
+// Arch <= ARMv8.3:  crypto = sha2 + aes
+// -
+//
+// Check +crypto:
+//
+// RUN: %clang -target aarch64 -march=armv8a+crypto -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-CRYPTO83 %s
+// RUN: %clang -target aarch64 -march=armv8.1a+crypto -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-CRYPTO83 %s
+// RUN: %clang -target aarch64 -march=armv8.2a+crypto -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-CRYPTO83 %s
+// RUN: %clang -target aarch64 -march=armv8.3a+crypto -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-CRYPTO83 %s
+// RUN: %clang -target aarch64 -march=armv8a+crypto+nocrypto+crypto -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-CRYPTO83 %s
+// CHECK-CRYPTO83: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-feature" "+crypto" "-target-feature" "+sha2" "-target-feature" "+aes"
+//
+// Check -crypto:
+//
+// RUN: %clang -target aarch64 -march=armv8a+nocrypto -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-NOCRYPTO8A %s
+// RUN: %clang -target aarch64 -march=armv8.1a+nocrypto -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-NOCRYPTO81 %s
+// RUN: %clang -target aarch64 -march=armv8.2a+nocrypto -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-NOCRYPTO82 %s
+// RUN: %clang -target aarch64 -march=armv8.3a+nocrypto -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-NOCRYPTO82 %s
+// RUN: %clang -target aarch64 -march=armv8.3a+nocrypto+crypto+nocrypto -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-NOCRYPTO82 %s
+
+// CHECK-NOCRYPTO8A: "-target-feature" "+neon" "-target-feature" "-crypto" "-target-feature" "-sha2" "-target-feature" "-aes" "-target-abi" "aapcs"
+// CHECK-NOCRYPTO81: "-target-feature" "+neon" "-target-feature" "+v8.1a" "-target-feature" "-crypto" "-target-feature" "-sha2" "-target-feature" "-aes" "-target-abi" "aapcs"
+// CHECK-NOCRYPTO82: "-target-feature" "+neon" "-target-feature" "+v8.{{.}}a" "-target-feature" "-crypto" "-target-feature" "-sha2" "-target-feature" "-aes" "-target-feature" "-sm4" "-target-feature" "-sha3" "-target-abi" "aapcs"
+//
+// Check +crypto -sha2 -aes:
+//
+// RUN: %clang -target aarch64 -march=armv8.1a+crypto+nosha2+noaes -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-CRYPTO83-NOSHA2-NOAES %s
+// CHECK-CRYPTO83-NOSHA2-NOAES-NOT: 

[PATCH] D50179: [AArch64][ARM] Context sensitive meaning of option "crypto"

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer added inline comments.



Comment at: lib/Driver/ToolChains/Arch/ARM.cpp:430
+  if (ArchName.find_lower("+noaes") == StringRef::npos)
+Features.push_back("+aes");
+} else if (ArchName.find_lower("-crypto") != StringRef::npos) {

efriedma wrote:
> The ARM backend doesn't support features named "sha2" and "aes" at the moment.
These ARM target features were introduced in rL335953.


https://reviews.llvm.org/D50179



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50175: [AArch64][NFC] better matching of AArch64 target in aarch64-cpus.c tests

[Sjoerd Meijer via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL339347: [AArch64][NFC] better matching of AArch64 target in 
aarch64-cpus.c tests (authored by SjoerdMeijer, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D50175?vs=159915=159920#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D50175

Files:
  cfe/trunk/test/Driver/aarch64-cpus.c

Index: cfe/trunk/test/Driver/aarch64-cpus.c
===
--- cfe/trunk/test/Driver/aarch64-cpus.c
+++ cfe/trunk/test/Driver/aarch64-cpus.c
@@ -6,7 +6,7 @@
 // RUN: %clang -target aarch64 -mlittle-endian -mcpu=generic -### -c %s 2>&1 | FileCheck -check-prefix=GENERIC %s
 // RUN: %clang -target aarch64_be -mlittle-endian -### -c %s 2>&1 | FileCheck -check-prefix=GENERIC %s
 // RUN: %clang -target aarch64_be -mlittle-endian -mcpu=generic -### -c %s 2>&1 | FileCheck -check-prefix=GENERIC %s
-// GENERIC: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-cpu" "generic"
+// GENERIC: "-cc1"{{.*}} "-triple" "aarch64{{(--)?}}"{{.*}} "-target-cpu" "generic"
 
 // RUN: %clang -target arm64 -### -c %s 2>&1 | FileCheck -check-prefix=ARM64-GENERIC %s
 // RUN: %clang -target arm64 -mcpu=generic -### -c %s 2>&1 | FileCheck -check-prefix=ARM64-GENERIC %s
@@ -29,8 +29,8 @@
 // RUN: %clang -target aarch64 -mtune=cortex-a35 -### -c %s 2>&1 | FileCheck -check-prefix=CA35-TUNE %s
 // RUN: %clang -target aarch64 -mlittle-endian -mtune=cortex-a35 -### -c %s 2>&1 | FileCheck -check-prefix=CA35-TUNE %s
 // RUN: %clang -target aarch64_be -mlittle-endian -mtune=cortex-a35 -### -c %s 2>&1 | FileCheck -check-prefix=CA35-TUNE %s
-// CA35: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-cpu" "cortex-a35"
-// CA35-TUNE: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-cpu" "generic"
+// CA35: "-cc1"{{.*}} "-triple" "aarch64{{(--)?}}"{{.*}} "-target-cpu" "cortex-a35"
+// CA35-TUNE: "-cc1"{{.*}} "-triple" "aarch64{{(--)?}}"{{.*}} "-target-cpu" "generic"
 
 // RUN: %clang -target arm64 -mcpu=cortex-a35 -### -c %s 2>&1 | FileCheck -check-prefix=ARM64-CA35 %s
 // RUN: %clang -target arm64 -mlittle-endian -mcpu=cortex-a35 -### -c %s 2>&1 | FileCheck -check-prefix=ARM64-CA35 %s
@@ -44,8 +44,8 @@
 // RUN: %clang -target aarch64_be -mlittle-endian -mcpu=cortex-a53 -### -c %s 2>&1 | FileCheck -check-prefix=CA53 %s
 // RUN: %clang -target aarch64 -mtune=cortex-a53 -### -c %s 2>&1 | FileCheck -check-prefix=CA53-TUNE %s
 // RUN: %clang -target aarch64_be -mlittle-endian -mtune=cortex-a53 -### -c %s 2>&1 | FileCheck -check-prefix=CA53-TUNE %s
-// CA53: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-cpu" "cortex-a53"
-// CA53-TUNE: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-cpu" "generic"
+// CA53: "-cc1"{{.*}} "-triple" "aarch64{{(--)?}}"{{.*}} "-target-cpu" "cortex-a53"
+// CA53-TUNE: "-cc1"{{.*}} "-triple" "aarch64{{(--)?}}"{{.*}} "-target-cpu" "generic"
 
 // RUN: %clang -target arm64 -mcpu=cortex-a53 -### -c %s 2>&1 | FileCheck -check-prefix=ARM64-CA53 %s
 // RUN: %clang -target arm64 -mlittle-endian -mcpu=cortex-a53 -### -c %s 2>&1 | FileCheck -check-prefix=ARM64-CA53 %s
@@ -59,8 +59,8 @@
 // RUN: %clang -target aarch64_be -mlittle-endian -mcpu=cortex-a55 -### -c %s 2>&1 | FileCheck -check-prefix=CA55 %s
 // RUN: %clang -target aarch64 -mtune=cortex-a55 -### -c %s 2>&1 | FileCheck -check-prefix=CA55-TUNE %s
 // RUN: %clang -target aarch64_be -mlittle-endian -mtune=cortex-a55 -### -c %s 2>&1 | FileCheck -check-prefix=CA55-TUNE %s
-// CA55: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-cpu" "cortex-a55"
-// CA55-TUNE: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-cpu" "generic"
+// CA55: "-cc1"{{.*}} "-triple" "aarch64{{(--)?}}"{{.*}} "-target-cpu" "cortex-a55"
+// CA55-TUNE: "-cc1"{{.*}} "-triple" "aarch64{{(--)?}}"{{.*}} "-target-cpu" "generic"
 
 // RUN: %clang -target arm64 -mcpu=cortex-a55 -### -c %s 2>&1 | FileCheck -check-prefix=ARM64-CA55 %s
 // RUN: %clang -target arm64 -mlittle-endian -mcpu=cortex-a55 -### -c %s 2>&1 | FileCheck -check-prefix=ARM64-CA55 %s
@@ -75,8 +75,8 @@
 // RUN: %clang -target aarch64 -mtune=cortex-a57 -### -c %s 2>&1 | FileCheck -check-prefix=CA57-TUNE %s
 // RUN: %clang -target aarch64 -mlittle-endian -mtune=cortex-a57 -### -c %s 2>&1 | FileCheck -check-prefix=CA57-TUNE %s
 // RUN: %clang -target aarch64_be -mlittle-endian -mtune=cortex-a57 -### -c %s 2>&1 | FileCheck -check-prefix=CA57-TUNE %s
-// CA57: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-cpu" "cortex-a57"
-// CA57-TUNE: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-cpu" "generic"
+// CA57: "-cc1"{{.*}} "-triple" "aarch64{{(--)?}}"{{.*}} "-target-cpu" "cortex-a57"
+// CA57-TUNE: "-cc1"{{.*}} "-triple" "aarch64{{(--)?}}"{{.*}} "-target-cpu" "generic"
 
 // RUN: %clang -target arm64 -mcpu=cortex-a57 -### -c %s 2>&1 | FileCheck -check-prefix=ARM64-CA57 %s
 // RUN: %clang -target arm64 -mlittle-endian -mcpu=cortex-a57 -### -c %s 2>&1 | FileCheck 

[PATCH] D50175: [AArch64][NFC] better matching of AArch64 target in aarch64-cpus.c tests

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer updated this revision to Diff 159915.
SjoerdMeijer added a comment.

Addressed comments.


https://reviews.llvm.org/D50175

Files:
  test/Driver/aarch64-cpus.c

Index: test/Driver/aarch64-cpus.c
===
--- test/Driver/aarch64-cpus.c
+++ test/Driver/aarch64-cpus.c
@@ -6,7 +6,7 @@
 // RUN: %clang -target aarch64 -mlittle-endian -mcpu=generic -### -c %s 2>&1 | FileCheck -check-prefix=GENERIC %s
 // RUN: %clang -target aarch64_be -mlittle-endian -### -c %s 2>&1 | FileCheck -check-prefix=GENERIC %s
 // RUN: %clang -target aarch64_be -mlittle-endian -mcpu=generic -### -c %s 2>&1 | FileCheck -check-prefix=GENERIC %s
-// GENERIC: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-cpu" "generic"
+// GENERIC: "-cc1"{{.*}} "-triple" "aarch64{{(--)?}}"{{.*}} "-target-cpu" "generic"
 
 // RUN: %clang -target arm64 -### -c %s 2>&1 | FileCheck -check-prefix=ARM64-GENERIC %s
 // RUN: %clang -target arm64 -mcpu=generic -### -c %s 2>&1 | FileCheck -check-prefix=ARM64-GENERIC %s
@@ -29,8 +29,8 @@
 // RUN: %clang -target aarch64 -mtune=cortex-a35 -### -c %s 2>&1 | FileCheck -check-prefix=CA35-TUNE %s
 // RUN: %clang -target aarch64 -mlittle-endian -mtune=cortex-a35 -### -c %s 2>&1 | FileCheck -check-prefix=CA35-TUNE %s
 // RUN: %clang -target aarch64_be -mlittle-endian -mtune=cortex-a35 -### -c %s 2>&1 | FileCheck -check-prefix=CA35-TUNE %s
-// CA35: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-cpu" "cortex-a35"
-// CA35-TUNE: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-cpu" "generic"
+// CA35: "-cc1"{{.*}} "-triple" "aarch64{{(--)?}}"{{.*}} "-target-cpu" "cortex-a35"
+// CA35-TUNE: "-cc1"{{.*}} "-triple" "aarch64{{(--)?}}"{{.*}} "-target-cpu" "generic"
 
 // RUN: %clang -target arm64 -mcpu=cortex-a35 -### -c %s 2>&1 | FileCheck -check-prefix=ARM64-CA35 %s
 // RUN: %clang -target arm64 -mlittle-endian -mcpu=cortex-a35 -### -c %s 2>&1 | FileCheck -check-prefix=ARM64-CA35 %s
@@ -44,8 +44,8 @@
 // RUN: %clang -target aarch64_be -mlittle-endian -mcpu=cortex-a53 -### -c %s 2>&1 | FileCheck -check-prefix=CA53 %s
 // RUN: %clang -target aarch64 -mtune=cortex-a53 -### -c %s 2>&1 | FileCheck -check-prefix=CA53-TUNE %s
 // RUN: %clang -target aarch64_be -mlittle-endian -mtune=cortex-a53 -### -c %s 2>&1 | FileCheck -check-prefix=CA53-TUNE %s
-// CA53: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-cpu" "cortex-a53"
-// CA53-TUNE: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-cpu" "generic"
+// CA53: "-cc1"{{.*}} "-triple" "aarch64{{(--)?}}"{{.*}} "-target-cpu" "cortex-a53"
+// CA53-TUNE: "-cc1"{{.*}} "-triple" "aarch64{{(--)?}}"{{.*}} "-target-cpu" "generic"
 
 // RUN: %clang -target arm64 -mcpu=cortex-a53 -### -c %s 2>&1 | FileCheck -check-prefix=ARM64-CA53 %s
 // RUN: %clang -target arm64 -mlittle-endian -mcpu=cortex-a53 -### -c %s 2>&1 | FileCheck -check-prefix=ARM64-CA53 %s
@@ -59,8 +59,8 @@
 // RUN: %clang -target aarch64_be -mlittle-endian -mcpu=cortex-a55 -### -c %s 2>&1 | FileCheck -check-prefix=CA55 %s
 // RUN: %clang -target aarch64 -mtune=cortex-a55 -### -c %s 2>&1 | FileCheck -check-prefix=CA55-TUNE %s
 // RUN: %clang -target aarch64_be -mlittle-endian -mtune=cortex-a55 -### -c %s 2>&1 | FileCheck -check-prefix=CA55-TUNE %s
-// CA55: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-cpu" "cortex-a55"
-// CA55-TUNE: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-cpu" "generic"
+// CA55: "-cc1"{{.*}} "-triple" "aarch64{{(--)?}}"{{.*}} "-target-cpu" "cortex-a55"
+// CA55-TUNE: "-cc1"{{.*}} "-triple" "aarch64{{(--)?}}"{{.*}} "-target-cpu" "generic"
 
 // RUN: %clang -target arm64 -mcpu=cortex-a55 -### -c %s 2>&1 | FileCheck -check-prefix=ARM64-CA55 %s
 // RUN: %clang -target arm64 -mlittle-endian -mcpu=cortex-a55 -### -c %s 2>&1 | FileCheck -check-prefix=ARM64-CA55 %s
@@ -75,8 +75,8 @@
 // RUN: %clang -target aarch64 -mtune=cortex-a57 -### -c %s 2>&1 | FileCheck -check-prefix=CA57-TUNE %s
 // RUN: %clang -target aarch64 -mlittle-endian -mtune=cortex-a57 -### -c %s 2>&1 | FileCheck -check-prefix=CA57-TUNE %s
 // RUN: %clang -target aarch64_be -mlittle-endian -mtune=cortex-a57 -### -c %s 2>&1 | FileCheck -check-prefix=CA57-TUNE %s
-// CA57: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-cpu" "cortex-a57"
-// CA57-TUNE: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-cpu" "generic"
+// CA57: "-cc1"{{.*}} "-triple" "aarch64{{(--)?}}"{{.*}} "-target-cpu" "cortex-a57"
+// CA57-TUNE: "-cc1"{{.*}} "-triple" "aarch64{{(--)?}}"{{.*}} "-target-cpu" "generic"
 
 // RUN: %clang -target arm64 -mcpu=cortex-a57 -### -c %s 2>&1 | FileCheck -check-prefix=ARM64-CA57 %s
 // RUN: %clang -target arm64 -mlittle-endian -mcpu=cortex-a57 -### -c %s 2>&1 | FileCheck -check-prefix=ARM64-CA57 %s
@@ -91,8 +91,8 @@
 // RUN: %clang -target aarch64 -mtune=cortex-a72 -### -c %s 2>&1 | FileCheck -check-prefix=CA72-TUNE %s
 // RUN: %clang -target aarch64 -mlittle-endian -mtune=cortex-a72 -### -c %s 2>&1 | FileCheck -check-prefix=CA72-TUNE %s
 // RUN: %clang -target aarch64_be 

[PATCH] D50175: [AArch64][NFC] better matching of AArch64 target in aarch64-cpus.c tests

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer added inline comments.



Comment at: test/Driver/aarch64-cpus.c:10
+// GENERIC: "-cc1"{{.*}} "-triple" "aarch64"{{.*}} "-target-cpu" "generic"
+// GENERIC-LE: "-cc1"{{.*}} "-triple" "aarch64--"{{.*}} "-target-cpu" "generic"
 

olista01 wrote:
> SjoerdMeijer wrote:
> > olista01 wrote:
> > > Why do these need new check prefixes? All of the RUN lines above are 
> > > selecting little-endian, so I'd expect GENERIC and GENERIC-LE to be the 
> > > same.
> > Ok, good point. The output is slightly different. For the little-endian 
> > runs above the output is:
> > 
> >   "-triple" "aarch64"
> > 
> > and with "-target aarch64_be -mlittle-endian" the output is:
> > 
> >   "-triple" "aarch64--"
> > 
> > As we don't want to be too generic and match "aarch64{{.*}}", I will 
> > therefore change the GENERIC checks to match "aarch64{{[--]*}}", and indeed 
> > remove GENERIC-LE.
> I think that works, but it's a strange way to write the regex. You have "-" 
> twice inside a character set, which is the same as only having it once, so 
> "[--]*" matches zero or more occurrences of "-". I'd suggest using something 
> like "(--)?" which matches either "--" or nothing.
Ah, of course, thanks! That was a bit silly, will fix.


https://reviews.llvm.org/D50175



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50175: [AArch64][NFC] better matching of AArch64 target in aarch64-cpus.c tests

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer updated this revision to Diff 159715.
SjoerdMeijer added a comment.

Addressed comments.


https://reviews.llvm.org/D50175

Files:
  test/Driver/aarch64-cpus.c

Index: test/Driver/aarch64-cpus.c
===
--- test/Driver/aarch64-cpus.c
+++ test/Driver/aarch64-cpus.c
@@ -6,7 +6,7 @@
 // RUN: %clang -target aarch64 -mlittle-endian -mcpu=generic -### -c %s 2>&1 | FileCheck -check-prefix=GENERIC %s
 // RUN: %clang -target aarch64_be -mlittle-endian -### -c %s 2>&1 | FileCheck -check-prefix=GENERIC %s
 // RUN: %clang -target aarch64_be -mlittle-endian -mcpu=generic -### -c %s 2>&1 | FileCheck -check-prefix=GENERIC %s
-// GENERIC: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-cpu" "generic"
+// GENERIC: "-cc1"{{.*}} "-triple" "aarch64{{[--]*}}"{{.*}} "-target-cpu" "generic"
 
 // RUN: %clang -target arm64 -### -c %s 2>&1 | FileCheck -check-prefix=ARM64-GENERIC %s
 // RUN: %clang -target arm64 -mcpu=generic -### -c %s 2>&1 | FileCheck -check-prefix=ARM64-GENERIC %s
@@ -29,8 +29,8 @@
 // RUN: %clang -target aarch64 -mtune=cortex-a35 -### -c %s 2>&1 | FileCheck -check-prefix=CA35-TUNE %s
 // RUN: %clang -target aarch64 -mlittle-endian -mtune=cortex-a35 -### -c %s 2>&1 | FileCheck -check-prefix=CA35-TUNE %s
 // RUN: %clang -target aarch64_be -mlittle-endian -mtune=cortex-a35 -### -c %s 2>&1 | FileCheck -check-prefix=CA35-TUNE %s
-// CA35: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-cpu" "cortex-a35"
-// CA35-TUNE: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-cpu" "generic"
+// CA35: "-cc1"{{.*}} "-triple" "aarch64{{[--]*}}"{{.*}} "-target-cpu" "cortex-a35"
+// CA35-TUNE: "-cc1"{{.*}} "-triple" "aarch64{{[--]*}}"{{.*}} "-target-cpu" "generic"
 
 // RUN: %clang -target arm64 -mcpu=cortex-a35 -### -c %s 2>&1 | FileCheck -check-prefix=ARM64-CA35 %s
 // RUN: %clang -target arm64 -mlittle-endian -mcpu=cortex-a35 -### -c %s 2>&1 | FileCheck -check-prefix=ARM64-CA35 %s
@@ -44,8 +44,8 @@
 // RUN: %clang -target aarch64_be -mlittle-endian -mcpu=cortex-a53 -### -c %s 2>&1 | FileCheck -check-prefix=CA53 %s
 // RUN: %clang -target aarch64 -mtune=cortex-a53 -### -c %s 2>&1 | FileCheck -check-prefix=CA53-TUNE %s
 // RUN: %clang -target aarch64_be -mlittle-endian -mtune=cortex-a53 -### -c %s 2>&1 | FileCheck -check-prefix=CA53-TUNE %s
-// CA53: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-cpu" "cortex-a53"
-// CA53-TUNE: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-cpu" "generic"
+// CA53: "-cc1"{{.*}} "-triple" "aarch64{{[--]*}}"{{.*}} "-target-cpu" "cortex-a53"
+// CA53-TUNE: "-cc1"{{.*}} "-triple" "aarch64{{[--]*}}"{{.*}} "-target-cpu" "generic"
 
 // RUN: %clang -target arm64 -mcpu=cortex-a53 -### -c %s 2>&1 | FileCheck -check-prefix=ARM64-CA53 %s
 // RUN: %clang -target arm64 -mlittle-endian -mcpu=cortex-a53 -### -c %s 2>&1 | FileCheck -check-prefix=ARM64-CA53 %s
@@ -59,8 +59,8 @@
 // RUN: %clang -target aarch64_be -mlittle-endian -mcpu=cortex-a55 -### -c %s 2>&1 | FileCheck -check-prefix=CA55 %s
 // RUN: %clang -target aarch64 -mtune=cortex-a55 -### -c %s 2>&1 | FileCheck -check-prefix=CA55-TUNE %s
 // RUN: %clang -target aarch64_be -mlittle-endian -mtune=cortex-a55 -### -c %s 2>&1 | FileCheck -check-prefix=CA55-TUNE %s
-// CA55: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-cpu" "cortex-a55"
-// CA55-TUNE: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-cpu" "generic"
+// CA55: "-cc1"{{.*}} "-triple" "aarch64{{[--]*}}"{{.*}} "-target-cpu" "cortex-a55"
+// CA55-TUNE: "-cc1"{{.*}} "-triple" "aarch64{{[--]*}}"{{.*}} "-target-cpu" "generic"
 
 // RUN: %clang -target arm64 -mcpu=cortex-a55 -### -c %s 2>&1 | FileCheck -check-prefix=ARM64-CA55 %s
 // RUN: %clang -target arm64 -mlittle-endian -mcpu=cortex-a55 -### -c %s 2>&1 | FileCheck -check-prefix=ARM64-CA55 %s
@@ -75,8 +75,8 @@
 // RUN: %clang -target aarch64 -mtune=cortex-a57 -### -c %s 2>&1 | FileCheck -check-prefix=CA57-TUNE %s
 // RUN: %clang -target aarch64 -mlittle-endian -mtune=cortex-a57 -### -c %s 2>&1 | FileCheck -check-prefix=CA57-TUNE %s
 // RUN: %clang -target aarch64_be -mlittle-endian -mtune=cortex-a57 -### -c %s 2>&1 | FileCheck -check-prefix=CA57-TUNE %s
-// CA57: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-cpu" "cortex-a57"
-// CA57-TUNE: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-cpu" "generic"
+// CA57: "-cc1"{{.*}} "-triple" "aarch64{{[--]*}}"{{.*}} "-target-cpu" "cortex-a57"
+// CA57-TUNE: "-cc1"{{.*}} "-triple" "aarch64{{[--]*}}"{{.*}} "-target-cpu" "generic"
 
 // RUN: %clang -target arm64 -mcpu=cortex-a57 -### -c %s 2>&1 | FileCheck -check-prefix=ARM64-CA57 %s
 // RUN: %clang -target arm64 -mlittle-endian -mcpu=cortex-a57 -### -c %s 2>&1 | FileCheck -check-prefix=ARM64-CA57 %s
@@ -91,8 +91,8 @@
 // RUN: %clang -target aarch64 -mtune=cortex-a72 -### -c %s 2>&1 | FileCheck -check-prefix=CA72-TUNE %s
 // RUN: %clang -target aarch64 -mlittle-endian -mtune=cortex-a72 -### -c %s 2>&1 | FileCheck -check-prefix=CA72-TUNE %s
 // RUN: %clang -target aarch64_be 

[PATCH] D50175: [AArch64][NFC] better matching of AArch64 target in aarch64-cpus.c tests

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer added inline comments.



Comment at: test/Driver/aarch64-cpus.c:10
+// GENERIC: "-cc1"{{.*}} "-triple" "aarch64"{{.*}} "-target-cpu" "generic"
+// GENERIC-LE: "-cc1"{{.*}} "-triple" "aarch64--"{{.*}} "-target-cpu" "generic"
 

olista01 wrote:
> Why do these need new check prefixes? All of the RUN lines above are 
> selecting little-endian, so I'd expect GENERIC and GENERIC-LE to be the same.
Ok, good point. The output is slightly different. For the little-endian runs 
above the output is:

  "-triple" "aarch64"

and with "-target aarch64_be -mlittle-endian" the output is:

  "-triple" "aarch64--"

As we don't want to be too generic and match "aarch64{{.*}}", I will therefore 
change the GENERIC checks to match "aarch64{{[--]*}}", and indeed remove 
GENERIC-LE.


https://reviews.llvm.org/D50175



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D49075: [NEON] Define fp16 vld and vst intrinsics conditionally

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer accepted this revision.
SjoerdMeijer added a comment.
This revision is now accepted and ready to land.

Thanks, LGTM.


https://reviews.llvm.org/D49075



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50179: [AArch64][ARM] Context sensitive meaning of option "crypto"

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer added a comment.

Hi Eli, thanks for the feedback.

> Yes, this logic should be in TargetParser, not here. Trying to rewrite the 
> target features afterwards is messy at best. (Actually, the target feature 
> list generated by TargetParser probably shouldn't contain the string "crypto" 
> at all.)

I appreciate there is room for improvement here, which is an understatement! :) 
I probably should have mentioned earlier that my colleague is working on 
targetparser and options, and he will send the proposal in the form of an RFC 
to the dev list soon. Very briefly, the proposal will elaborate on how we want 
to capture/enforce architecture extension dependencies (I believe thus also 
disallow architecturally invalid combinations), imply options, and e.g. warn on 
redundant options.

I want to move the crypto logic to this new framework as soon it is there. 
Thus, for the time being, this is a stopgap to demonstrate what we want to 
achieve (with crypto), and also quite importantly, we have something that works 
today. But again, I fully agree that the current implementation is far from 
ideal, but hopefully with these explanations is somewhat acceptable.


https://reviews.llvm.org/D50179



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50179: [AArch64][ARM] Context sensitive meaning of option "crypto"

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer created this revision.
SjoerdMeijer added reviewers: olista01, samparker, john.brawn, ab, 
t.p.northover.
Herald added a reviewer: javed.absar.
Herald added subscribers: chrib, kristof.beyls.

For AArch64:

1. Crypto means sm4 + sha3 + sha2 + aes for Armv8.4-A and up,
2. and sha2 + aes for Armv8.3-A and earlier.

And for AArch32:
Crypto means sha2 + aes, because the Armv8.2-A crypto instructions
were added to AArch64 only.


https://reviews.llvm.org/D50179

Files:
  lib/Driver/ToolChains/Arch/AArch64.cpp
  lib/Driver/ToolChains/Arch/ARM.cpp
  test/Driver/arm-features.c
  test/Preprocessor/aarch64-target-features.c

Index: test/Preprocessor/aarch64-target-features.c
===
--- test/Preprocessor/aarch64-target-features.c
+++ test/Preprocessor/aarch64-target-features.c
@@ -143,6 +143,101 @@
 // CHECK-MARCH-2: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-feature" "-fp-armv8" "-target-feature" "-neon" "-target-feature" "-crc" "-target-feature" "-crypto"
 // CHECK-MARCH-3: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-feature" "-neon"
 
+// Check +sm4:
+//
+// RUN: %clang -target aarch64 -march=armv8.2a+sm4 -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-SM4 %s
+// CHECK-SM4:  "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-feature" "+v8.2a" "-target-feature" "+sm4"
+//
+// Check +sha3:
+//
+// RUN: %clang -target aarch64 -march=armv8.2a+sha3 -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-SHA3 %s
+// CHECK-SHA3: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-feature" "+v8.2a" "-target-feature" "+sha3"
+//
+// Check +sha2:
+//
+// RUN: %clang -target aarch64 -march=armv8.3a+sha2 -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-SHA2 %s
+// CHECK-SHA2: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-feature" "+v8.{{.}}a" "-target-feature" "+sha2"
+//
+// Check +aes:
+//
+// RUN: %clang -target aarch64 -march=armv8.3a+aes -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-AES %s
+// CHECK-AES: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-feature" "+v8.{{.}}a" "-target-feature" "+aes"
+//
+// Check -sm4:
+//
+// RUN: %clang -target aarch64 -march=armv8.2a+noSM4 -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-NO-SM4 %s
+// CHECK-NO-SM4:  "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-feature" "+v8.2a" "-target-feature" "-sm4"
+//
+// Check -sha3:
+//
+// RUN: %clang -target aarch64 -march=armv8.2a+noSHA3 -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-NO-SHA3 %s
+// CHECK-NO-SHA3: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-feature" "+v8.2a" "-target-feature" "-sha3"
+//
+// Check -sha2:
+//
+// RUN: %clang -target aarch64 -march=armv8.2a+noSHA2 -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-NO-SHA2 %s
+// CHECK-NO-SHA2: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-feature" "+v8.2a" "-target-feature" "-sha2"
+//
+// Check -aes:
+//
+// RUN: %clang -target aarch64 -march=armv8.2a+noAES -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-NO-AES %s
+// CHECK-NO-AES: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-feature" "+v8.2a" "-target-feature" "-aes"
+//
+//
+// Arch <= ARMv8.3:  crypto = sha2 + aes
+// -
+//
+// Check +crypto:
+//
+// RUN: %clang -target aarch64 -march=armv8a+crypto -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-CRYPTO83 %s
+// RUN: %clang -target aarch64 -march=armv8.1a+crypto -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-CRYPTO83 %s
+// RUN: %clang -target aarch64 -march=armv8.2a+crypto -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-CRYPTO83 %s
+// RUN: %clang -target aarch64 -march=armv8.3a+crypto -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-CRYPTO83 %s
+// RUN: %clang -target aarch64 -march=armv8a+crypto+nocrypto+crypto -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-CRYPTO83 %s
+// CHECK-CRYPTO83: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-feature" "+crypto" "-target-feature" "+sha2" "-target-feature" "+aes"
+//
+// Check -crypto:
+//
+// RUN: %clang -target aarch64 -march=armv8a+nocrypto -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-NOCRYPTO8A %s
+// RUN: %clang -target aarch64 -march=armv8.1a+nocrypto -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-NOCRYPTO81 %s
+// RUN: %clang -target aarch64 -march=armv8.2a+nocrypto -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-NOCRYPTO82 %s
+// RUN: %clang -target aarch64 -march=armv8.3a+nocrypto -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-NOCRYPTO82 %s
+// RUN: %clang -target aarch64 -march=armv8.3a+nocrypto+crypto+nocrypto -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-NOCRYPTO82 %s
+
+// CHECK-NOCRYPTO8A: "-target-feature" "+neon" "-target-feature" "-crypto" "-target-feature" "-sha2" "-target-feature" "-aes" "-target-abi" "aapcs"
+// CHECK-NOCRYPTO81: "-target-feature" "+neon" "-target-feature" "+v8.1a" "-target-feature" "-crypto" "-target-feature" "-sha2" "-target-feature" "-aes" "-target-abi" "aapcs"
+// CHECK-NOCRYPTO82: "-target-feature" "+neon" "-target-feature" "+v8.{{.}}a" "-target-feature" "-crypto" 

[PATCH] D50175: [AArch64][NFC] better matching of AArch64 target in aarch64-cpus.c tests

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer created this revision.
SjoerdMeijer added reviewers: olista01, ab.
Herald added a reviewer: javed.absar.
Herald added a subscriber: kristof.beyls.

In https://reviews.llvm.org/D50068, @ab noticed that it would be better to 
match aarch64-{{.*}} for
tests that use "-target aarch64_be -mlittle-endian" instead of just
aarch64{{.*}}.


https://reviews.llvm.org/D50175

Files:
  test/Driver/aarch64-cpus.c

Index: test/Driver/aarch64-cpus.c
===
--- test/Driver/aarch64-cpus.c
+++ test/Driver/aarch64-cpus.c
@@ -4,9 +4,10 @@
 // RUN: %clang -target aarch64 -mcpu=generic -### -c %s 2>&1 | FileCheck -check-prefix=GENERIC %s
 // RUN: %clang -target aarch64 -mlittle-endian -### -c %s 2>&1 | FileCheck -check-prefix=GENERIC %s
 // RUN: %clang -target aarch64 -mlittle-endian -mcpu=generic -### -c %s 2>&1 | FileCheck -check-prefix=GENERIC %s
-// RUN: %clang -target aarch64_be -mlittle-endian -### -c %s 2>&1 | FileCheck -check-prefix=GENERIC %s
-// RUN: %clang -target aarch64_be -mlittle-endian -mcpu=generic -### -c %s 2>&1 | FileCheck -check-prefix=GENERIC %s
-// GENERIC: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-cpu" "generic"
+// RUN: %clang -target aarch64_be -mlittle-endian -### -c %s 2>&1 | FileCheck -check-prefix=GENERIC-LE %s
+// RUN: %clang -target aarch64_be -mlittle-endian -mcpu=generic -### -c %s 2>&1 | FileCheck -check-prefix=GENERIC-LE %s
+// GENERIC: "-cc1"{{.*}} "-triple" "aarch64"{{.*}} "-target-cpu" "generic"
+// GENERIC-LE: "-cc1"{{.*}} "-triple" "aarch64--"{{.*}} "-target-cpu" "generic"
 
 // RUN: %clang -target arm64 -### -c %s 2>&1 | FileCheck -check-prefix=ARM64-GENERIC %s
 // RUN: %clang -target arm64 -mcpu=generic -### -c %s 2>&1 | FileCheck -check-prefix=ARM64-GENERIC %s
@@ -25,12 +26,14 @@
 
 // RUN: %clang -target aarch64 -mcpu=cortex-a35 -### -c %s 2>&1 | FileCheck -check-prefix=CA35 %s
 // RUN: %clang -target aarch64 -mlittle-endian -mcpu=cortex-a35 -### -c %s 2>&1 | FileCheck -check-prefix=CA35 %s
-// RUN: %clang -target aarch64_be -mlittle-endian -mcpu=cortex-a35 -### -c %s 2>&1 | FileCheck -check-prefix=CA35 %s
+// RUN: %clang -target aarch64_be -mlittle-endian -mcpu=cortex-a35 -### -c %s 2>&1 | FileCheck -check-prefix=CA35-LE %s
 // RUN: %clang -target aarch64 -mtune=cortex-a35 -### -c %s 2>&1 | FileCheck -check-prefix=CA35-TUNE %s
 // RUN: %clang -target aarch64 -mlittle-endian -mtune=cortex-a35 -### -c %s 2>&1 | FileCheck -check-prefix=CA35-TUNE %s
-// RUN: %clang -target aarch64_be -mlittle-endian -mtune=cortex-a35 -### -c %s 2>&1 | FileCheck -check-prefix=CA35-TUNE %s
-// CA35: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-cpu" "cortex-a35"
-// CA35-TUNE: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-cpu" "generic"
+// RUN: %clang -target aarch64_be -mlittle-endian -mtune=cortex-a35 -### -c %s 2>&1 | FileCheck -check-prefix=CA35-TUNE-LE %s
+// CA35: "-cc1"{{.*}} "-triple" "aarch64"{{.*}} "-target-cpu" "cortex-a35"
+// CA35-LE: "-cc1"{{.*}} "-triple" "aarch64--"{{.*}} "-target-cpu" "cortex-a35"
+// CA35-TUNE: "-cc1"{{.*}} "-triple" "aarch64"{{.*}} "-target-cpu" "generic"
+// CA35-TUNE-LE: "-cc1"{{.*}} "-triple" "aarch64--"{{.*}} "-target-cpu" "generic"
 
 // RUN: %clang -target arm64 -mcpu=cortex-a35 -### -c %s 2>&1 | FileCheck -check-prefix=ARM64-CA35 %s
 // RUN: %clang -target arm64 -mlittle-endian -mcpu=cortex-a35 -### -c %s 2>&1 | FileCheck -check-prefix=ARM64-CA35 %s
@@ -41,11 +44,13 @@
 
 // RUN: %clang -target aarch64 -mcpu=cortex-a53 -### -c %s 2>&1 | FileCheck -check-prefix=CA53 %s
 // RUN: %clang -target aarch64 -mlittle-endian -mcpu=cortex-a53 -### -c %s 2>&1 | FileCheck -check-prefix=CA53 %s
-// RUN: %clang -target aarch64_be -mlittle-endian -mcpu=cortex-a53 -### -c %s 2>&1 | FileCheck -check-prefix=CA53 %s
+// RUN: %clang -target aarch64_be -mlittle-endian -mcpu=cortex-a53 -### -c %s 2>&1 | FileCheck -check-prefix=CA53-LE %s
 // RUN: %clang -target aarch64 -mtune=cortex-a53 -### -c %s 2>&1 | FileCheck -check-prefix=CA53-TUNE %s
-// RUN: %clang -target aarch64_be -mlittle-endian -mtune=cortex-a53 -### -c %s 2>&1 | FileCheck -check-prefix=CA53-TUNE %s
-// CA53: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-cpu" "cortex-a53"
-// CA53-TUNE: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-cpu" "generic"
+// RUN: %clang -target aarch64_be -mlittle-endian -mtune=cortex-a53 -### -c %s 2>&1 | FileCheck -check-prefix=CA53-TUNE-LE %s
+// CA53: "-cc1"{{.*}} "-triple" "aarch64"{{.*}} "-target-cpu" "cortex-a53"
+// CA53-LE: "-cc1"{{.*}} "-triple" "aarch64--"{{.*}} "-target-cpu" "cortex-a53"
+// CA53-TUNE: "-cc1"{{.*}} "-triple" "aarch64"{{.*}} "-target-cpu" "generic"
+// CA53-TUNE-LE: "-cc1"{{.*}} "-triple" "aarch64--"{{.*}} "-target-cpu" "generic"
 
 // RUN: %clang -target arm64 -mcpu=cortex-a53 -### -c %s 2>&1 | FileCheck -check-prefix=ARM64-CA53 %s
 // RUN: %clang -target arm64 -mlittle-endian -mcpu=cortex-a53 -### -c %s 2>&1 | FileCheck -check-prefix=ARM64-CA53 %s
@@ -56,11 +61,13 @@
 
 // RUN: 

[PATCH] D50068: [AArch64][ARM] Add Armv8.4-A tests

[Sjoerd Meijer via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC338525: [AArch64][ARM] Add Armv8.4-A tests (authored by 
SjoerdMeijer, committed by ).

Repository:
  rC Clang

https://reviews.llvm.org/D50068

Files:
  lib/Basic/Targets/ARM.cpp
  test/Driver/aarch64-cpus.c
  test/Driver/arm-cortex-cpus.c

Index: lib/Basic/Targets/ARM.cpp
===
--- lib/Basic/Targets/ARM.cpp
+++ lib/Basic/Targets/ARM.cpp
@@ -185,6 +185,10 @@
 return "8_1A";
   case llvm::ARM::ArchKind::ARMV8_2A:
 return "8_2A";
+  case llvm::ARM::ArchKind::ARMV8_3A:
+return "8_3A";
+  case llvm::ARM::ArchKind::ARMV8_4A:
+return "8_4A";
   case llvm::ARM::ArchKind::ARMV8MBaseline:
 return "8M_BASE";
   case llvm::ARM::ArchKind::ARMV8MMainline:
Index: test/Driver/aarch64-cpus.c
===
--- test/Driver/aarch64-cpus.c
+++ test/Driver/aarch64-cpus.c
@@ -388,6 +388,14 @@
 // RUN: %clang -target aarch64_be -mlittle-endian -march=armv8.2-a -### -c %s 2>&1 | FileCheck -check-prefix=GENERICV82A %s
 // GENERICV82A: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-cpu" "generic" "-target-feature" "+neon" "-target-feature" "+v8.2a"
 
+// RUN: %clang -target aarch64_be -march=armv8.2a -### -c %s 2>&1 | FileCheck -check-prefix=GENERICV82A-BE %s
+// RUN: %clang -target aarch64_be -march=armv8.2-a -### -c %s 2>&1 | FileCheck -check-prefix=GENERICV82A-BE %s
+// RUN: %clang -target aarch64 -mbig-endian -march=armv8.2a -### -c %s 2>&1 | FileCheck -check-prefix=GENERICV82A-BE %s
+// RUN: %clang -target aarch64 -mbig-endian -march=armv8.2-a -### -c %s 2>&1 | FileCheck -check-prefix=GENERICV82A-BE %s
+// RUN: %clang -target aarch64_be -mbig-endian -march=armv8.2a -### -c %s 2>&1 | FileCheck -check-prefix=GENERICV82A-BE %s
+// RUN: %clang -target aarch64_be -mbig-endian -march=armv8.2-a -### -c %s 2>&1 | FileCheck -check-prefix=GENERICV82A-BE %s
+// GENERICV82A-BE: "-cc1"{{.*}} "-triple" "aarch64_be{{.*}}" "-target-cpu" "generic" "-target-feature" "+neon" "-target-feature" "+v8.2a"
+
 // RUN: %clang -target aarch64 -march=armv8.2-a+fp16 -### -c %s 2>&1 | FileCheck -check-prefix=GENERICV82A-FP16 %s
 // GENERICV82A-FP16: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-cpu" "generic" "-target-feature" "+neon" "-target-feature" "+v8.2a" "-target-feature" "+fullfp16"
 
@@ -397,6 +405,71 @@
 // RUN: %clang -target aarch64 -march=armv8.2-a+fp16+profile -### -c %s 2>&1 | FileCheck -check-prefix=GENERICV82A-FP16-SPE %s
 // GENERICV82A-FP16-SPE: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-cpu" "generic" "-target-feature" "+neon" "-target-feature" "+v8.2a" "-target-feature" "+fullfp16" "-target-feature" "+spe"
 
+// RUN: %clang -target aarch64 -march=armv8.3a -### -c %s 2>&1 | FileCheck -check-prefix=GENERICV83A %s
+// RUN: %clang -target aarch64 -march=armv8.3-a -### -c %s 2>&1 | FileCheck -check-prefix=GENERICV83A %s
+// RUN: %clang -target aarch64 -mlittle-endian -march=armv8.3a -### -c %s 2>&1 | FileCheck -check-prefix=GENERICV83A %s
+// RUN: %clang -target aarch64 -mlittle-endian -march=armv8.3-a -### -c %s 2>&1 | FileCheck -check-prefix=GENERICV83A %s
+// RUN: %clang -target aarch64_be -mlittle-endian -march=armv8.3a -### -c %s 2>&1 | FileCheck -check-prefix=GENERICV83A %s
+// RUN: %clang -target aarch64_be -mlittle-endian -march=armv8.3-a -### -c %s 2>&1 | FileCheck -check-prefix=GENERICV83A %s
+// GENERICV83A: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-cpu" "generic" "-target-feature" "+neon" "-target-feature" "+v8.3a"
+
+// RUN: %clang -target aarch64_be -march=armv8.3a -### -c %s 2>&1 | FileCheck -check-prefix=GENERICV83A-BE %s
+// RUN: %clang -target aarch64_be -march=armv8.3-a -### -c %s 2>&1 | FileCheck -check-prefix=GENERICV83A-BE %s
+// RUN: %clang -target aarch64 -mbig-endian -march=armv8.3a -### -c %s 2>&1 | FileCheck -check-prefix=GENERICV83A-BE %s
+// RUN: %clang -target aarch64 -mbig-endian -march=armv8.3-a -### -c %s 2>&1 | FileCheck -check-prefix=GENERICV83A-BE %s
+// RUN: %clang -target aarch64_be -mbig-endian -march=armv8.3a -### -c %s 2>&1 | FileCheck -check-prefix=GENERICV83A-BE %s
+// RUN: %clang -target aarch64_be -mbig-endian -march=armv8.3-a -### -c %s 2>&1 | FileCheck -check-prefix=GENERICV83A-BE %s
+// GENERICV83A-BE: "-cc1"{{.*}} "-triple" "aarch64_be{{.*}}" "-target-cpu" "generic" "-target-feature" "+neon" "-target-feature" "+v8.3a"
+
+// RUN: %clang -target aarch64 -march=armv8.3-a+fp16 -### -c %s 2>&1 | FileCheck -check-prefix=GENERICV83A-FP16 %s
+// GENERICV83A-FP16: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-cpu" "generic" "-target-feature" "+neon" "-target-feature" "+v8.3a" "-target-feature" "+fullfp16"
+
+// RUN: %clang -target aarch64 -march=armv8.4a -### -c %s 2>&1 | FileCheck -check-prefix=GENERICV84A %s
+// RUN: %clang -target aarch64 -march=armv8.4-a -### -c %s 2>&1 | FileCheck -check-prefix=GENERICV84A %s
+// RUN: %clang -target aarch64 

[PATCH] D50068: [AArch64][ARM] Add Armv8.4-A tests

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer added inline comments.



Comment at: test/Driver/aarch64-cpus.c:9
 // RUN: %clang -target aarch64_be -mlittle-endian -mcpu=generic -### -c %s 
2>&1 | FileCheck -check-prefix=GENERIC %s
 // GENERIC: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-cpu" "generic"
 

ab wrote:
> Ideally, these should test `aarch64-{{.*}}`, no?
Agreed, good point. I will commit this first, and address this in a follow up 
as it looks like needs fixing in a few places here in this file. 


https://reviews.llvm.org/D50068



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50068: [AArch64][ARM] Add Armv8.4-A tests

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer created this revision.
SjoerdMeijer added reviewers: samparker, olista01, john.brawn, ab, 
t.p.northover.
Herald added a reviewer: javed.absar.
Herald added subscribers: chrib, kristof.beyls.

This adds tests for Armv8.4-A, and also some v8.2 and v8.3 tests that were 
missed in previous upstreaming exercises.


https://reviews.llvm.org/D50068

Files:
  lib/Basic/Targets/ARM.cpp
  test/Driver/aarch64-cpus.c
  test/Driver/arm-cortex-cpus.c

Index: test/Driver/arm-cortex-cpus.c
===
--- test/Driver/arm-cortex-cpus.c
+++ test/Driver/arm-cortex-cpus.c
@@ -287,13 +287,62 @@
 // RUN: %clang -target armv8a-linux-eabi -march=armv8.2-a+fp16 -### -c %s 2>&1 | FileCheck --check-prefix CHECK-V82A-FP16 %s
 // CHECK-V82A-FP16: "-cc1"{{.*}} "-triple" "armv8.2{{.*}}" "-target-cpu" "generic" {{.*}}"-target-feature" "+fullfp16"
 
+// RUN: %clang -target armv8.3a -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-V83A %s
+// RUN: %clang -target arm -march=armv8.3a -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-V83A %s
+// RUN: %clang -target arm -march=armv8.3-a -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-V83A %s
+// RUN: %clang -target arm -march=armv8.3a -mlittle-endian -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-V83A %s
+// RUN: %clang -target armv8.3a -mlittle-endian -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-V83A %s
+// RUN: %clang -target arm -march=armv8.3a -mlittle-endian -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-V83A %s
+// RUN: %clang -target arm -mlittle-endian -march=armv8.3-a -mlittle-endian -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-V83A %s
+// CHECK-V83A: "-cc1"{{.*}} "-triple" "armv8.3{{.*}}" "-target-cpu" "generic"
+
+// RUN: %clang -target armebv8.3a -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-BE-V83A %s
+// RUN: %clang -target armv8.3a -mbig-endian -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-BE-V83A %s
+// RUN: %clang -target armeb -march=armebv8.3a -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-BE-V83A %s
+// RUN: %clang -target armeb -march=armebv8.3-a -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-BE-V83A %s
+// RUN: %clang -target arm -march=armebv8.3a -mbig-endian -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-BE-V83A %s
+// RUN: %clang -target arm -march=armebv8.3-a -mbig-endian -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-BE-V83A %s
+// CHECK-BE-V83A: "-cc1"{{.*}} "-triple" "armebv8.3{{.*}}" "-target-cpu" "generic"
+
+// RUN: %clang -target armv8a-linux-eabi -march=armv8.3-a+fp16 -### -c %s 2>&1 | FileCheck --check-prefix CHECK-V83A-FP16 %s
+// CHECK-V83A-FP16: "-cc1"{{.*}} "-triple" "armv8.3{{.*}}" "-target-cpu" "generic" {{.*}}"-target-feature" "+fullfp16"
+
+// RUN: %clang -target armv8.4a -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-V84A %s
+// RUN: %clang -target arm -march=armv8.4a -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-V84A %s
+// RUN: %clang -target arm -march=armv8.4-a -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-V84A %s
+// RUN: %clang -target arm -march=armv8.4a -mlittle-endian -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-V84A %s
+// RUN: %clang -target armv8.4a -mlittle-endian -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-V84A %s
+// RUN: %clang -target arm -march=armv8.4a -mlittle-endian -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-V84A %s
+// RUN: %clang -target arm -mlittle-endian -march=armv8.4-a -mlittle-endian -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-V84A %s
+// CHECK-V84A: "-cc1"{{.*}} "-triple" "armv8.4{{.*}}" "-target-cpu" "generic"
+
+// RUN: %clang -target armebv8.4a -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-BE-V84A %s
+// RUN: %clang -target armv8.4a -mbig-endian -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-BE-V84A %s
+// RUN: %clang -target armeb -march=armebv8.4a -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-BE-V84A %s
+// RUN: %clang -target armeb -march=armebv8.4-a -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-BE-V84A %s
+// RUN: %clang -target arm -march=armebv8.4a -mbig-endian -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-BE-V84A %s
+// RUN: %clang -target arm -march=armebv8.4-a -mbig-endian -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-BE-V84A %s
+// CHECK-BE-V84A: "-cc1"{{.*}} "-triple" "armebv8.4{{.*}}" "-target-cpu" "generic"
+
+// RUN: %clang -target armv8a-linux-eabi -march=armv8.4-a+fp16 -### -c %s 2>&1 | FileCheck --check-prefix CHECK-V84A-FP16 %s
+// CHECK-V84A-FP16: "-cc1"{{.*}} "-triple" "armv8.4{{.*}}" "-target-cpu" "generic" {{.*}}"-target-feature" "+fullfp16"
+
 // Once we have CPUs with optional v8.2-A FP16, we will need a way to turn it
 // on and off. Cortex-A53 is a placeholder for now.
 // RUN: %clang -target armv8a-linux-eabi -mcpu=cortex-a53+fp16 -### -c %s 2>&1 | FileCheck --check-prefix CHECK-CORTEX-A53-FP16 %s
 // RUN: %clang -target armv8a-linux-eabi -mcpu=cortex-a53+nofp16 -### -c %s 2>&1 | FileCheck --check-prefix CHECK-CORTEX-A53-NOFP16 %s
 // CHECK-CORTEX-A53-FP16: "-cc1" 

[PATCH] D49376: [NEON] Define half-precision vrnd intrinsics only when available

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer added inline comments.



Comment at: include/clang/Basic/arm_neon.td:1419
   // Vector rounding
-  def FRINTZH  : SInst<"vrnd",  "dd", "hQh">;
-  def FRINTNH  : SInst<"vrndn", "dd", "hQh">;
-  def FRINTAH  : SInst<"vrnda", "dd", "hQh">;
-  def FRINTPH  : SInst<"vrndp", "dd", "hQh">;
-  def FRINTMH  : SInst<"vrndm", "dd", "hQh">;
-  def FRINTXH  : SInst<"vrndx", "dd", "hQh">;
+  let ArchGuard = "__ARM_ARCH >= 8 && defined(__ARM_FEATURE_DIRECTED_ROUNDING) 
&& defined(__ARM_FEATURE_FP16_VECTOR_ARITHMETIC)" in {
+def FRINTZH  : SInst<"vrnd",  "dd", "hQh">;

kosarev wrote:
> SjoerdMeijer wrote:
> > nit: is the indentation a bit off here?
> It's a nested `let ArchGuard`, so I guess we do want the indentation here?
Yep, got confused, please ignore.


https://reviews.llvm.org/D49376



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D49375: [NEON] Define half-precision vmaxnm intrinsics only when available

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer added inline comments.



Comment at: include/clang/Basic/arm_neon.td:1466
   def VMINH : SInst<"vmin", "ddd", "hQh">;
-  def FMAXNMH   : SInst<"vmaxnm", "ddd", "hQh">;
-  def FMINNMH   : SInst<"vminnm", "ddd", "hQh">;
+  let ArchGuard = "__ARM_ARCH >= 8 && defined(__ARM_FEATURE_NUMERIC_MAXMIN) && 
defined(__ARM_FEATURE_FP16_VECTOR_ARITHMETIC)" in {
+def FMAXNMH   : SInst<"vmaxnm", "ddd", "hQh">;

kosarev wrote:
> SjoerdMeijer wrote:
> > nit: indentation?
> Do we want some special indentation here?
Ah, sorry, got confused, it's just one big string. 


https://reviews.llvm.org/D49375



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D48829: [NEON] Fix support for vrndi_f32(), vrndiq_f32() and vrndns_f32() intrinsics

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer accepted this revision.
SjoerdMeijer added a comment.
This revision is now accepted and ready to land.

LGTM


https://reviews.llvm.org/D48829



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D49075: [NEON] Define fp16 vld and vst intrinsics conditionally

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer added a comment.

Now that they are conditionally defined, do we need negative tests (in 
test/Sema/arm-no-fp16.c?) to check that they are not available when fp16 is not 
enabled?


https://reviews.llvm.org/D49075



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D49375: [NEON] Define half-precision vmaxnm intrinsics only when available

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer accepted this revision.
SjoerdMeijer added a comment.
This revision is now accepted and ready to land.

LGTM




Comment at: include/clang/Basic/arm_neon.td:1466
   def VMINH : SInst<"vmin", "ddd", "hQh">;
-  def FMAXNMH   : SInst<"vmaxnm", "ddd", "hQh">;
-  def FMINNMH   : SInst<"vminnm", "ddd", "hQh">;
+  let ArchGuard = "__ARM_ARCH >= 8 && defined(__ARM_FEATURE_NUMERIC_MAXMIN) && 
defined(__ARM_FEATURE_FP16_VECTOR_ARITHMETIC)" in {
+def FMAXNMH   : SInst<"vmaxnm", "ddd", "hQh">;

nit: indentation?


https://reviews.llvm.org/D49375



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D49376: [NEON] Define half-precision vrnd intrinsics only when available

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer accepted this revision.
SjoerdMeijer added a comment.
This revision is now accepted and ready to land.

LGTM




Comment at: include/clang/Basic/arm_neon.td:1419
   // Vector rounding
-  def FRINTZH  : SInst<"vrnd",  "dd", "hQh">;
-  def FRINTNH  : SInst<"vrndn", "dd", "hQh">;
-  def FRINTAH  : SInst<"vrnda", "dd", "hQh">;
-  def FRINTPH  : SInst<"vrndp", "dd", "hQh">;
-  def FRINTMH  : SInst<"vrndm", "dd", "hQh">;
-  def FRINTXH  : SInst<"vrndx", "dd", "hQh">;
+  let ArchGuard = "__ARM_ARCH >= 8 && defined(__ARM_FEATURE_DIRECTED_ROUNDING) 
&& defined(__ARM_FEATURE_FP16_VECTOR_ARITHMETIC)" in {
+def FRINTZH  : SInst<"vrnd",  "dd", "hQh">;

nit: is the indentation a bit off here?


https://reviews.llvm.org/D49376



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D48440: [NEON] Support vldNq intrinsics in AArch32 (Clang part)

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer accepted this revision.
SjoerdMeijer added a comment.
This revision is now accepted and ready to land.

Looks OK to me.
But file "test/CodeGen/vld_dup.c" looks weird/empty. I guess you're removing it?


https://reviews.llvm.org/D48440



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D48188: [SPIR] Prevent SPIR targets from using half conversion intrinsics

[Sjoerd Meijer via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC335111: [SPIR] Prevent SPIR targets from using half 
conversion intrinsics (authored by SjoerdMeijer, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D48188?vs=151938=152045#toc

Repository:
  rC Clang

https://reviews.llvm.org/D48188

Files:
  lib/Basic/Targets/SPIR.h
  test/CodeGen/spir-half-type.cpp

Index: test/CodeGen/spir-half-type.cpp
===
--- test/CodeGen/spir-half-type.cpp
+++ test/CodeGen/spir-half-type.cpp
@@ -0,0 +1,146 @@
+// RUN: %clang_cc1 -O0 -triple spir -emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1 -O0 -triple spir64 -emit-llvm %s -o - | FileCheck %s
+
+// This file tests that using the _Float16 type with the spir target will not
+// use the llvm intrinsics but instead will use the half arithmetic
+// instructions directly.
+
+// Previously attempting to use a constant _Float16 with a comparison
+// instruction when the target is spir or spir64 lead to an assert being hit.
+bool fcmp_const() {
+  _Float16 a = 0.0f16;
+  const _Float16 b = 1.0f16;
+
+  // CHECK-NOT: llvm.convert.to.fp16
+  // CHECK-NOT: llvm.convert.from.fp16
+
+  // CHECK: [[REG1:%.*]] = load half, half* %a, align 2
+  // CHECK-NEXT: fcmp olt half [[REG1]], 0xH3C00
+
+  // CHECK: [[REG2:%.*]] = load half, half* %a, align 2
+  // CHECK-NEXT: fcmp olt half [[REG2]], 0xH4000
+
+  // CHECK: [[REG3:%.*]] = load half, half* %a, align 2
+  // CHECK-NEXT: fcmp ogt half [[REG3]], 0xH3C00
+
+  // CHECK: [[REG4:%.*]] = load half, half* %a, align 2
+  // CHECK-NEXT: fcmp ogt half [[REG4]], 0xH4200
+
+  // CHECK: [[REG5:%.*]] = load half, half* %a, align 2
+  // CHECK-NEXT: fcmp oeq half [[REG5]], 0xH3C00
+
+  // CHECK: [[REG7:%.*]] = load half, half* %a, align 2
+  // CHECK-NEXT: fcmp oeq half [[REG7]], 0xH4400
+
+  // CHECK: [[REG8:%.*]] = load half, half* %a, align 2
+  // CHECK-NEXT: fcmp une half [[REG8]], 0xH3C00
+
+  // CHECK: [[REG9:%.*]] = load half, half* %a, align 2
+  // CHECK-NEXT: fcmp une half [[REG9]], 0xH4500
+
+  // CHECK: [[REG10:%.*]] = load half, half* %a, align 2
+  // CHECK-NEXT: fcmp ole half [[REG10]], 0xH3C00
+
+  // CHECK: [[REG11:%.*]] = load half, half* %a, align 2
+  // CHECK-NEXT: fcmp ole half [[REG11]], 0xH4600
+
+  // CHECK: [[REG12:%.*]] = load half, half* %a, align 2
+  // CHECK-NEXT: fcmp oge half [[REG12]], 0xH3C00
+
+  // CHECK: [[REG13:%.*]] = load half, half* %a, align 2
+  // CHECK-NEXT: fcmp oge half [[REG13]], 0xH4700
+  return a < b || a < 2.0f16 || a > b || a > 3.0f16 || a == b || a == 4.0f16 ||
+ a != b || a != 5.0f16 || a <= b || a <= 6.0f16 || a >= b ||
+ a >= 7.0f16;
+}
+
+bool fcmp() {
+  _Float16 a = 0.0f16;
+  _Float16 b = 1.0f16;
+
+  // CHECK-NOT: llvm.convert.to.fp16
+  // CHECK-NOT: llvm.convert.from.fp16
+  // CHECK: [[REG1:%.*]] = load half, half* %a, align 2
+  // CHECK-NEXT: [[REG2:%.*]] = load half, half* %b, align 2
+  // CHECK-NEXT: fcmp olt half [[REG1]], [[REG2]]
+
+  // CHECK: [[REG3:%.*]] = load half, half* %a, align 2
+  // CHECK-NEXT: [[REG4:%.*]] = load half, half* %b, align 2
+  // CHECK-NEXT: fcmp ogt half [[REG3]], [[REG4]]
+
+  // CHECK: [[REG5:%.*]] = load half, half* %a, align 2
+  // CHECK-NEXT: [[REG6:%.*]] = load half, half* %b, align 2
+  // CHECK-NEXT: fcmp oeq half [[REG5]], [[REG6]]
+
+  // CHECK: [[REG7:%.*]] = load half, half* %a, align 2
+  // CHECK-NEXT: [[REG8:%.*]] = load half, half* %b, align 2
+  // CHECK-NEXT: fcmp une half [[REG7]], [[REG8]]
+
+  // CHECK: [[REG7:%.*]] = load half, half* %a, align 2
+  // CHECK-NEXT: [[REG8:%.*]] = load half, half* %b, align 2
+  // CHECK-NEXT: fcmp ole half [[REG7]], [[REG8]]
+
+  // CHECK: [[REG7:%.*]] = load half, half* %a, align 2
+  // CHECK-NEXT: [[REG8:%.*]] = load half, half* %b, align 2
+  // CHECK-NEXT: fcmp oge half [[REG7]], [[REG8]]
+  return a < b || a > b || a == b || a != b || a <= b || a >= b;
+}
+
+_Float16 fadd() {
+  _Float16 a = 1.0f16;
+  const _Float16 b = 2.0f16;
+
+  // CHECK-NOT: llvm.convert.to.fp16
+  // CHECK-NOT: llvm.convert.from.fp16
+
+  // CHECK: [[REG1:%.*]] = load half, half* %a, align 2
+  // CHECK-NEXT: [[REG2:%.*]] = fadd half [[REG1]], 0xH4000
+  // CHECK-NEXT: [[REG3:%.*]] = fadd half [[REG2]], 0xH4200
+  // CHECK-NEXT: ret half [[REG3]]
+  return a + b + 3.0f16;
+}
+
+_Float16 fsub() {
+  _Float16 a = 1.0f16;
+  const _Float16 b = 2.0f16;
+
+  // CHECK-NOT: llvm.convert.to.fp16
+  // CHECK-NOT: llvm.convert.from.fp16
+
+  // CHECK: [[REG1:%.*]] = load half, half* %a, align 2
+  // CHECK-NEXT: [[REG2:%.*]] = fsub half [[REG1]], 0xH4000
+  // CHECK-NEXT: [[REG3:%.*]] = fsub half [[REG2]], 0xH4200
+  // CHECK-NEXT: ret half [[REG3]]
+  return a - b - 3.0f16;
+}
+
+// CHECK: define spir_func half @_Z4fmulDF16_(half %arg)
+_Float16 fmul(_Float16 arg) {
+  _Float16 a = 1.0f16;
+  const _Float16 b = 2.0f16;
+
+  // CHECK-NOT: llvm.convert.to.fp16
+  // CHECK-NOT: 

[PATCH] D48188: [SPIR] Prevent SPIR targets from using half conversion intrinsics

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer added a comment.

No problem, will commit this today.


https://reviews.llvm.org/D48188



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D48188: [SPIR] Prevent SPIR targets from using half conversion intrinsics

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer accepted this revision.
SjoerdMeijer added a comment.
This revision is now accepted and ready to land.

Looks OK to me.




Comment at: test/CodeGen/spir-half-type.cpp:89
+
+_Float16 fadd() {
+  _Float16 a = 1.0f16;

Nit: let one of these functions take a _Float16 function argument?


https://reviews.llvm.org/D48188



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D48188: [SPIR] Prevent SPIR targets from using half conversion intrinsics

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer added a comment.

I know very little about SPIR, and Initially didn't understand this:

> The SPIR target currently allows for half precision floating point types to 
> use the LLVM intrinsic functions to convert to floats and doubles. This is 
> illegal in SPIR as the only intrinsic allowed by SPIR is memcpy ...

until I looked at the implementation what you're trying to achieve here. 
Perhaps you can make the commit message a bit more descriptive and specific.




Comment at: lib/Basic/Targets/SPIR.h:50
 UseAddrSpaceMapMangling = true;
+HasLegalHalfType = true;
 // Define available target features

It doesn't hurt to set this, but you're not using it so you could omit it. I 
had to introduce this to deal differently with half types depending on 
architecture extensions, but don't you think have this problem.



Comment at: lib/Basic/Targets/SPIR.h:65
+  // memcpy as per section 3 of the SPIR spec.
+  bool useFP16ConversionIntrinsics() const override { return false; }
+

just a note: this is the only functional change, but you're testing a lot more 
in test/CodeGen/spir-half-type.cpp



Comment at: test/CodeGen/spir-half-type.cpp:3
+// RUN: %clang_cc1 -O0 -triple spir64 -emit-llvm %s -o - | FileCheck %s
+
+// This file tests that using the _Float16 type with the spir target will not 
use the llvm intrinsics but instead will use the half arithmetic instructions 
directly.

I think you need one reproducer to test:

// CHECK-NOT: llvm.convert.from.fp16

The other tests, like all the compares are valid tests, but not related to this 
change, and also not specific to SPIR. I put my _Float16 "smoke tests" in 
test/CodeGenCXX/float16-declarations.cpp, perhaps you can move some of these 
generic tests there because I for example see I didn't add any compares there. 



Comment at: test/CodeGen/spir-half-type.cpp:4
+
+// This file tests that using the _Float16 type with the spir target will not 
use the llvm intrinsics but instead will use the half arithmetic instructions 
directly.
+

nit: this comment exceeds 80 columns, same for the other comment below.


Repository:
  rC Clang

https://reviews.llvm.org/D48188



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits