[PATCH] D127812: [AArch64] Function multiversioning support added.

[Sam Tebbs via Phabricator via cfe-commits]

samtebbs added inline comments.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:11483
+def warn_target_clone_no_impact_options
+: Warning<"version list contains no code impact entries">,
+  InGroup;

ilinpv wrote:
> erichkeane wrote:
> > I'm not clear as to what this means?
> It gives a warning if target_clones attributes contains features which have 
> no impact on code generation ( no supported yet ) and ignored. They has 
> "" OPTION in llvm/include/llvm/Support/AArch64TargetParser.def 
> See clang/test/Sema/attr-target-clones-aarch64.c tests
> ```
> // expected-warning@+1 {{version list contains no code impact entries}}
> void __attribute__((target_clones("sha1+pmull"))) warn2(void);
> 
> // expected-warning@+1 {{version list contains no code impact entries}}
> int __attribute__((target_clones("rng", "fhm+dpb+sha1", "default"))) 
> redecl4(void) { return 1; }
> ```
Perhaps it would be better to warn with something like "version list contains 
entries that don't impact code generation". I think that would be more clear.



Comment at: clang/include/clang/Basic/TargetInfo.h:1311
+  /// generation.
+  virtual bool isCodeImpactFeatureName(StringRef Feature) const { return true; 
}
+

Similarly to the warning string, I think a name like 
`featureAffectsCodeGen(...)` would be more clear in its use.



Comment at: clang/lib/Basic/Targets/AArch64.cpp:663
+if (Feature == "-fmv") {
+  HasFMV = false;
+}

Slight nit, but braces aren't needed here.



Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:2561
   } else if (!TargetDecl->isMultiVersion() &&
- TargetDecl->hasAttr()) {
+ (TargetDecl->hasAttr())) {
 // Get the required features for the callee.

Parentheses not needed.



Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:2664
+  default:
+assert(false && "Only implemented for x86 and AArch64 targets");
+  }

I'm not 100% sure on the differences between `assert(..)` and 
`LLvm_unreachable(...)`, but perhaps that would be cleaner than an assertion on 
`false`.



Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:2670
+llvm::Function *Resolver, ArrayRef Options) {
+  assert(!Options.empty() && "No Multiversion Resolver Options Found");
+  assert(Options.back().Conditions.Features.size() == 0 &&

Nit on the capital letters for every word. Only "No" requires capitalisation.



Comment at: clang/lib/Sema/SemaDecl.cpp:10497
+  (TA || TVA) &&
+  "MultiVersion Candidate requires a target or target_version attribute");
   const TargetInfo &TargetInfo = S.Context.getTargetInfo();

Candidate doesn't need capitalisation either.



Comment at: clang/lib/Sema/SemaDeclAttr.cpp:3521
 bool DefaultIsDupe = false;
+bool IsCodeImpact = false;
 if (Cur.empty())

I think `HasCodeImpact` would be more explanatory.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127812

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

[PATCH] D96007: [AArch64] Enable stack clash protection for AArch64 linux in clang

[Sam Tebbs via Phabricator via cfe-commits]

samtebbs added inline comments.
Herald added a subscriber: MaskRay.
Herald added a project: All.



Comment at: clang/lib/CodeGen/CodeGenModule.cpp:1669-1672
+if (CodeGenOpts.StackProbeSize != 4096)
+  B.addAttribute("stack-probe-size",
+ llvm::utostr(CodeGenOpts.StackProbeSize));
+  }

Would it be worth always setting the `stack-probe-size` attribute so that it is 
always correct and to avoid the risk of it going out-of-sync in case of changes 
to the assumed size in the backend?


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

https://reviews.llvm.org/D96007

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

[PATCH] D158008: [AArch64] Add patterns for FMADD, FMSUB

[Sam Tebbs via Phabricator via cfe-commits]

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

Thanks, this looks good to me with any extra tests required by David's comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158008

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

[PATCH] D146644: [documentation]Fixed Random Typos

[Sam Tebbs via Phabricator via cfe-commits]

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

Thank you for this, it looks like it fixes all of the typos reported in the 
issue. If you don't have commit access yet I can commit this with you as the 
author.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146644

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

[PATCH] D146644: [documentation]Fixed Random Typos

[Sam Tebbs via Phabricator via cfe-commits]

samtebbs added a comment.

I was about to commit it for you but think that "Random" should be removed from 
the title, are you OK with that?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146644

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

[PATCH] D146644: [documentation]Fixed Random Typos

[Sam Tebbs via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG4071dcf3549f: [documentation] Fix Typos (authored by 
ipriyanshi1708, committed by samtebbs).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146644

Files:
  llvm/include/llvm/Support/CommandLine.h
  llvm/lib/Support/FileUtilities.cpp
  llvm/test/DebugInfo/macro_link.ll
  llvm/tools/llvm-cfi-verify/llvm-cfi-verify.cpp


Index: llvm/tools/llvm-cfi-verify/llvm-cfi-verify.cpp
===
--- llvm/tools/llvm-cfi-verify/llvm-cfi-verify.cpp
+++ llvm/tools/llvm-cfi-verify/llvm-cfi-verify.cpp
@@ -7,7 +7,7 @@
 
//===--===//
 //
 // This tool verifies Control Flow Integrity (CFI) instrumentation by static
-// binary anaylsis. See the design document in /docs/CFIVerify.rst for more
+// binary analysis. See the design document in /docs/CFIVerify.rst for more
 // information.
 //
 // This tool is currently incomplete. It currently only does disassembly for
Index: llvm/test/DebugInfo/macro_link.ll
===
--- llvm/test/DebugInfo/macro_link.ll
+++ llvm/test/DebugInfo/macro_link.ll
@@ -1,6 +1,6 @@
 ; RUN: llvm-link %s %s -S -o -| FileCheck %s
 
-; This test checks that DIMacro and DIMacroFile comaprison works correctly.
+; This test checks that DIMacro and DIMacroFile comparison works correctly.
 
 ; CHECK: !llvm.dbg.cu = !{[[CU1:![0-9]*]], [[CU2:![0-9]*]]}
 
Index: llvm/lib/Support/FileUtilities.cpp
===
--- llvm/lib/Support/FileUtilities.cpp
+++ llvm/lib/Support/FileUtilities.cpp
@@ -169,7 +169,7 @@
 
 /// DiffFilesWithTolerance - Compare the two files specified, returning 0 if 
the
 /// files match, 1 if they are different, and 2 if there is a file error.  This
-/// function differs from DiffFiles in that you can specify an absolete and
+/// function differs from DiffFiles in that you can specify an absolute and
 /// relative FP error that is allowed to exist.  If you specify a string to 
fill
 /// in for the error option, it will set the string to an error message if an
 /// error occurs, allowing the caller to distinguish between a failed diff and 
a
Index: llvm/include/llvm/Support/CommandLine.h
===
--- llvm/include/llvm/Support/CommandLine.h
+++ llvm/include/llvm/Support/CommandLine.h
@@ -220,7 +220,7 @@
   static SubCommand &getTopLevel();
 
   // Get the special subcommand that can be used to put an option into all
-  // subcomands.
+  // subcommands.
   static SubCommand &getAll();
 
   void reset();


Index: llvm/tools/llvm-cfi-verify/llvm-cfi-verify.cpp
===
--- llvm/tools/llvm-cfi-verify/llvm-cfi-verify.cpp
+++ llvm/tools/llvm-cfi-verify/llvm-cfi-verify.cpp
@@ -7,7 +7,7 @@
 //===--===//
 //
 // This tool verifies Control Flow Integrity (CFI) instrumentation by static
-// binary anaylsis. See the design document in /docs/CFIVerify.rst for more
+// binary analysis. See the design document in /docs/CFIVerify.rst for more
 // information.
 //
 // This tool is currently incomplete. It currently only does disassembly for
Index: llvm/test/DebugInfo/macro_link.ll
===
--- llvm/test/DebugInfo/macro_link.ll
+++ llvm/test/DebugInfo/macro_link.ll
@@ -1,6 +1,6 @@
 ; RUN: llvm-link %s %s -S -o -| FileCheck %s
 
-; This test checks that DIMacro and DIMacroFile comaprison works correctly.
+; This test checks that DIMacro and DIMacroFile comparison works correctly.
 
 ; CHECK: !llvm.dbg.cu = !{[[CU1:![0-9]*]], [[CU2:![0-9]*]]}
 
Index: llvm/lib/Support/FileUtilities.cpp
===
--- llvm/lib/Support/FileUtilities.cpp
+++ llvm/lib/Support/FileUtilities.cpp
@@ -169,7 +169,7 @@
 
 /// DiffFilesWithTolerance - Compare the two files specified, returning 0 if the
 /// files match, 1 if they are different, and 2 if there is a file error.  This
-/// function differs from DiffFiles in that you can specify an absolete and
+/// function differs from DiffFiles in that you can specify an absolute and
 /// relative FP error that is allowed to exist.  If you specify a string to fill
 /// in for the error option, it will set the string to an error message if an
 /// error occurs, allowing the caller to distinguish between a failed diff and a
Index: llvm/include/llvm/Support/CommandLine.h
===
--- llvm/include/llvm/Support/CommandLine.h
+++ llvm/include/llvm/Support/CommandLine.h
@@ -220,7 +220,7 @@
   static SubCommand &getTopLevel();
 
   // Get the 

[PATCH] D146892: [documentation]Fixed Random Typo

[Sam Tebbs via Phabricator via cfe-commits]

samtebbs added a comment.

Thank you Ayushi. Since you don't have commit access yet, I can commit it for 
you if you'd like. I do wonder if we should change the commit title to be 
something like "[documentation] Fix some typos". What do you think?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146892

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

[PATCH] D146370: [Clang][OpenMP]Solved the the always truth condition in Arm64

[Sam Tebbs via Phabricator via cfe-commits]

samtebbs added a comment.

Thank you Samuel. Since you don't have commit access, I can commit this for you 
if you'd like.


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

https://reviews.llvm.org/D146370

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

[PATCH] D147197: [llvm-docs] Added documentation for the 'neg' operation

[Sam Tebbs via Phabricator via cfe-commits]

samtebbs added a comment.

Thank you oyalesalami. It looks like these instructions don't actually exist 
anymore so shouldn't be documented.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147197

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

[PATCH] D146892: [documentation] Fix some typos

[Sam Tebbs via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGcc8237d9d727: [documentation] Fix some typos (authored by 
ayushi-8102, committed by samtebbs).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146892

Files:
  clang/docs/AutomaticReferenceCounting.rst
  clang/docs/ConstantInterpreter.rst
  clang/docs/CrossCompilation.rst
  clang/docs/DataFlowAnalysisIntro.md
  clang/docs/DebuggingCoroutines.rst
  clang/docs/analyzer/developer-docs/nullability.rst
  clang/include/clang/AST/CXXInheritance.h
  clang/include/clang/AST/CommentSema.h
  clang/include/clang/AST/Decl.h
  clang/include/clang/AST/DeclBase.h
  clang/include/clang/AST/DeclTemplate.h
  clang/include/clang/AST/DeclarationName.h
  clang/include/clang/AST/Expr.h
  clang/include/clang/Analysis/Analyses/CalledOnceCheck.h
  clang/include/clang/Analysis/Analyses/Consumed.h

Index: clang/include/clang/Analysis/Analyses/Consumed.h
===
--- clang/include/clang/Analysis/Analyses/Consumed.h
+++ clang/include/clang/Analysis/Analyses/Consumed.h
@@ -258,7 +258,7 @@
 /// Check a function's CFG for consumed violations.
 ///
 /// We traverse the blocks in the CFG, keeping track of the state of each
-/// value who's type has uniquness annotations.  If methods are invoked in
+/// value who's type has uniqueness annotations.  If methods are invoked in
 /// the wrong state a warning is issued.  Each block in the CFG is traversed
 /// exactly once.
 void run(AnalysisDeclContext &AC);
Index: clang/include/clang/Analysis/Analyses/CalledOnceCheck.h
===
--- clang/include/clang/Analysis/Analyses/CalledOnceCheck.h
+++ clang/include/clang/Analysis/Analyses/CalledOnceCheck.h
@@ -28,7 +28,7 @@
 /// \enum IfThen -- then branch of the if statement has no call.
 /// \enum IfElse -- else branch of the if statement has no call.
 /// \enum Switch -- one of the switch cases doesn't have a call.
-/// \enum SwitchSkipped -- there is no call if none of the cases appies.
+/// \enum SwitchSkipped -- there is no call if none of the cases applies.
 /// \enum LoopEntered -- no call when the loop is entered.
 /// \enum LoopSkipped -- no call when the loop is not entered.
 /// \enum FallbackReason -- fallback case when we were not able to figure out
Index: clang/include/clang/AST/Expr.h
===
--- clang/include/clang/AST/Expr.h
+++ clang/include/clang/AST/Expr.h
@@ -135,8 +135,8 @@
   void setDependence(ExprDependence Deps) {
 ExprBits.Dependent = static_cast(Deps);
   }
-  friend class ASTImporter; // Sets dependence dircetly.
-  friend class ASTStmtReader; // Sets dependence dircetly.
+  friend class ASTImporter;   // Sets dependence directly.
+  friend class ASTStmtReader; // Sets dependence directly.
 
 public:
   QualType getType() const { return TR; }
@@ -171,7 +171,7 @@
   }
 
   /// Determines whether the type of this expression depends on
-  ///   - a template paramter (C++ [temp.dep.expr], which means that its type
+  ///   - a template parameter (C++ [temp.dep.expr], which means that its type
   /// could change from one template instantiation to the next)
   ///   - or an error
   ///
@@ -820,7 +820,7 @@
   /// member expression.
   static QualType findBoundMemberType(const Expr *expr);
 
-  /// Skip past any invisble AST nodes which might surround this
+  /// Skip past any invisible AST nodes which might surround this
   /// statement, such as ExprWithCleanups or ImplicitCastExpr nodes,
   /// but also injected CXXMemberExpr and CXXConstructExpr which represent
   /// implicit conversions.
@@ -924,7 +924,7 @@
 return const_cast(this)->IgnoreParenLValueCasts();
   }
 
-  /// Skip past any parenthese and casts which do not change the value
+  /// Skip past any parentheses and casts which do not change the value
   /// (including ptr->int casts of the same size) until reaching a fixed point.
   /// Skips:
   /// * What IgnoreParens() skips
@@ -2815,7 +2815,7 @@
   /// The number of arguments in the call expression.
   unsigned NumArgs;
 
-  /// The location of the right parenthese. This has a different meaning for
+  /// The location of the right parentheses. This has a different meaning for
   /// the derived classes of CallExpr.
   SourceLocation RParenLoc;
 
Index: clang/include/clang/AST/DeclarationName.h
===
--- clang/include/clang/AST/DeclarationName.h
+++ clang/include/clang/AST/DeclarationName.h
@@ -763,7 +763,7 @@
 };
 
 /// DeclarationNameInfo - A collector data type for bundling together
-/// a DeclarationName and the correspnding source/type location info.
+/// a DeclarationName and the corresponding source/type location info.
 struct DeclarationNameInfo {
 priva

[PATCH] D147197: [llvm-docs] Added documentation for the 'neg' operation

[Sam Tebbs via Phabricator via cfe-commits]

samtebbs added a comment.

Since the instruction's don't exist now, would you mind abandoning this 
revision?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147197

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

[PATCH] D147989: [clang] Fix Attribute Placement

[Sam Tebbs via Phabricator via cfe-commits]

samtebbs added inline comments.



Comment at: clang/lib/Sema/SemaDecl.cpp:58
 using namespace sema;
-
+bool isEnumClass = false;
 Sema::DeclGroupPtrTy Sema::ConvertDeclToDeclGroup(Decl *Ptr, Decl *OwnedType) {

This being global could lead to issues where one overwrites the other. I think 
a better approach would be to create another version of 
`GetDiagnosticTypeSpecifierID` that takes the DeclSpec, then this won't be 
needed.



Comment at: clang/lib/Sema/SemaDecl.cpp:5305-5310
+   if (TypeSpecType == DeclSpec::TST_enum) {
+if (const EnumDecl *ED = dyn_cast(DS.getRepAsDecl())) {
+  if (ED->isScopedUsingClassTag())
+isEnumClass = true;
+}
+  }

if you make a new `GetDiagnosticTypeSpecifierID` then you can check these 
conditions in that function instead of there and return 5 if they are all true, 
otherwise return `GetDiagnosticTypeSpecifierID(TypeSpecType)`.



Comment at: clang/lib/Sema/SemaDecl.cpp:5313-5316
 << AL << GetDiagnosticTypeSpecifierID(TypeSpecType);
   for (const ParsedAttr &AL : DeclAttrs)
 Diag(AL.getLoc(), diag::warn_declspec_attribute_ignored)
 << AL << GetDiagnosticTypeSpecifierID(TypeSpecType);

Then use the new `GetDiagnosticTypeSpecifierID` here, passing `DS` instead of 
`TypeSpecType`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147989

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

[PATCH] D147989: [clang] Fix Attribute Placement

[Sam Tebbs via Phabricator via cfe-commits]

samtebbs added reviewers: mboehme, aaron.ballman.
samtebbs added a comment.

Looping in some people who have worked on and reviewed patches in this area for 
some experienced eyes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147989

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

[PATCH] D147989: [clang] Fix Attribute Placement

[Sam Tebbs via Phabricator via cfe-commits]

samtebbs added a comment.

Thanks for the input Aaron and Martin. This is looking good. With the suggested 
changes (don't forget the test, you could try adding one to 
clang/test/Sema/attr-declspec-ignored.c and 
clang/test/SemaCXX/attr-declspec-ignored.cpp) and some formatting 
 I think it will 
be ready.




Comment at: clang/include/clang/Sema/DeclSpec.h:274
   };
-
+  
   // Import type specifier type enumeration and constants.

There is still a whitespace change here.



Comment at: clang/lib/Sema/SemaDecl.cpp:5045
 }
-
+static unsigned GetDiagnosticTypeSpecifierIDWithAttrs(DeclSpec::TST T, const 
DeclSpec &DS){
+  if (T == DeclSpec::TST_enum) {

Thanks to C++ overloading, this can be called `GetDiagnosticTypeSpecifierID`, 
and I don't think it will need the `T` parameter since that can be fetched with 
`DS.getTypeSpecType()`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147989

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

[PATCH] D147989: [clang] Fix Attribute Placement

[Sam Tebbs via Phabricator via cfe-commits]

samtebbs added inline comments.



Comment at: clang/lib/Sema/SemaDecl.cpp:5051
+return 5;
+  else
+return 4;

Instead of returning 4 here, I think it's best to just delegate to the other 
`GetDiagnosticTypeSpecifierID` function. That way, if the IDs change for 
whatever reason they only have to updated in one place. This could be done 
cleanly by just making the call below be at the top level of the function (i.e. 
not inside an `else` block).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147989

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

[PATCH] D147989: [clang] Fix Attribute Placement

[Sam Tebbs via Phabricator via cfe-commits]

samtebbs added inline comments.



Comment at: clang/lib/Sema/SemaDecl.cpp:5041
   case DeclSpec::TST_enum:
 return 4;
   default:

ipriyanshi1708 wrote:
> jrtc27 wrote:
> > Why not just always pass the full DeclSpec and handle the class case here, 
> > maybe with a bool to allow other users to not need to treat enum class as 
> > different?
> Ya, we can do that too. But like GetDiagnosticTypeSpecifierID() is also 
> called in many other places in the file so if I pass the full DeclSpec in 
> this function will not disturb those calls?
It's probably worth doing as the other callsites just pass the `DeclSpec`'s 
type specifier.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147989

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

[PATCH] D147989: [clang] Fix Attribute Placement

[Sam Tebbs via Phabricator via cfe-commits]

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

This looks good to me now, nice work. Let's wait a few days for others' input 
to be safe.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147989

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