[PATCH] D142733: Add _Optional as fast qualifier

[River Riddle via Phabricator via cfe-commits]

rriddle added a comment.

I don't understand the MLIR changes here, how are they relevant to the patch?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142733

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

[PATCH] D134087: [TableGen] Track reference locations of Records/RecordVals

[River Riddle via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
rriddle marked 2 inline comments as done.
Closed by commit rG50d96f59d096: [TableGen] Track reference locations of 
Records/RecordVals (authored by rriddle).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Changed prior to commit:
  https://reviews.llvm.org/D134087?vs=461503=463440#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134087

Files:
  clang/test/TableGen/redefined-group.td
  llvm/include/llvm/TableGen/Record.h
  llvm/lib/TableGen/TGLexer.cpp
  llvm/lib/TableGen/TGLexer.h
  llvm/lib/TableGen/TGParser.cpp
  llvm/lib/TableGen/TGParser.h
  llvm/test/TableGen/ConstraintChecking1.td
  llvm/test/TableGen/ConstraintChecking2.td
  llvm/test/TableGen/ConstraintChecking3.td
  llvm/test/TableGen/ConstraintChecking4.td
  llvm/test/TableGen/ConstraintChecking5.td
  llvm/test/TableGen/ConstraintChecking6.td
  llvm/test/TableGen/ConstraintChecking7.td
  llvm/test/TableGen/GlobalISelEmitter-setcc.td
  llvm/test/TableGen/RegisterClass.td
  llvm/test/TableGen/SchedModelError.td
  llvm/test/TableGen/generic-tables.td
  mlir/lib/Tools/tblgen-lsp-server/TableGenServer.cpp

Index: mlir/lib/Tools/tblgen-lsp-server/TableGenServer.cpp
===
--- mlir/lib/Tools/tblgen-lsp-server/TableGenServer.cpp
+++ mlir/lib/Tools/tblgen-lsp-server/TableGenServer.cpp
@@ -202,16 +202,15 @@
 // Add references to the definition.
 for (SMLoc loc : def.getLoc().drop_front())
   insertRef(sym, lsp::convertTokenLocToRange(loc));
-
-// Add references to any super classes.
-for (auto  : def.getSuperClasses())
-  insertRef(getOrInsertDef(it.first),
-lsp::convertTokenLocToRange(it.second.Start));
+for (SMRange loc : def.getReferenceLocs())
+  insertRef(sym, loc);
 
 // Add definitions for any values.
 for (const llvm::RecordVal  : def.getValues()) {
   auto *sym = getOrInsertDef();
   insertRef(sym, sym->defLoc, /*isDef=*/true);
+  for (SMRange refLoc : value.getReferenceLocs())
+insertRef(sym, refLoc);
 }
   }
 }
Index: llvm/test/TableGen/generic-tables.td
===
--- llvm/test/TableGen/generic-tables.td
+++ llvm/test/TableGen/generic-tables.td
@@ -154,7 +154,7 @@
 }
 
 def DFoo : DEntry<"foo", 1>;
-// ERROR1: [[@LINE+1]]:1: error: Record 'DBar' for table 'DTable' is missing field 'Val1'
+// ERROR1: [[@LINE+1]]:5: error: Record 'DBar' for table 'DTable' is missing field 'Val1'
 def DBar : DEntry<"bar", ?>;
 
 def DTable : GenericTable {
Index: llvm/test/TableGen/SchedModelError.td
===
--- llvm/test/TableGen/SchedModelError.td
+++ llvm/test/TableGen/SchedModelError.td
@@ -4,7 +4,7 @@
 
 def TestTarget : Target;
 
-// CHECK: [[FILE]]:[[@LINE+1]]:1: error: No schedule information for instruction 'TestInst' in SchedMachineModel 'TestSchedModel'
+// CHECK: [[FILE]]:[[@LINE+1]]:5: error: No schedule information for instruction 'TestInst' in SchedMachineModel 'TestSchedModel'
 def TestInst : Instruction {
   let OutOperandList = (outs);
   let InOperandList = (ins);
Index: llvm/test/TableGen/RegisterClass.td
===
--- llvm/test/TableGen/RegisterClass.td
+++ llvm/test/TableGen/RegisterClass.td
@@ -4,4 +4,4 @@
 
 def MyTarget : Target;
 def R0 : Register<"r0">;
-def ClassA : RegisterClass<"MyTarget", [], 32, (add R0)>; // CHECK: [[@LINE]]:1: error: RegTypes list must not be empty!
+def ClassA : RegisterClass<"MyTarget", [], 32, (add R0)>; // CHECK: [[@LINE]]:5: error: RegTypes list must not be empty!
Index: llvm/test/TableGen/GlobalISelEmitter-setcc.td
===
--- llvm/test/TableGen/GlobalISelEmitter-setcc.td
+++ llvm/test/TableGen/GlobalISelEmitter-setcc.td
@@ -19,6 +19,6 @@
[(set GPR32:$dst, (i32 (setcc i32:$src0, i32:$src1, SETEQ)))]>;
 
 // Check there is an error if not a CondCode operand.
-// ERR: [[FILE]]:[[@LINE+1]]:1: warning: Skipped pattern: Unable to handle CondCode
+// ERR: [[FILE]]:[[@LINE+1]]:5: warning: Skipped pattern: Unable to handle CondCode
 def FCMP_NOTCC : I<(outs GPR32:$dst), (ins FPR32Op:$src0, FPR32:$src1),
[(set GPR32:$dst, (i32 (setcc f32:$src0, f32:$src1, i32)))]>;
Index: llvm/test/TableGen/ConstraintChecking7.td
===
--- llvm/test/TableGen/ConstraintChecking7.td
+++ llvm/test/TableGen/ConstraintChecking7.td
@@ -2,5 +2,5 @@
 
 include "ConstraintChecking.inc"
 
-// CHECK: [[FILE]]:[[@LINE+1]]:1: error: Operand '$src1' of 'Foo' cannot have multiple constraints!
+// CHECK: [[FILE]]:[[@LINE+1]]:5: error: Operand 

[PATCH] D124750: [MLIR] Add a utility to sort the operands of commutative ops

[River Riddle via Phabricator via cfe-commits]

rriddle added a comment.

+1 on all of the other comments, especially related to the use of strings.

In D124750#3503607 , @Mogball wrote:

> On the matter of whether this should be a canonicalization, my concern with 
> this is that if an operation has its own preferred ordering of operands that 
> conflicts with the sort, then this will cause canonicalization to loop 
> infinitely.
>
> It's not actually the canonicalizer pass that moves constants to the right 
> hand size. It's the folder. And it probably shouldn't be the folder that does 
> this. So I'm open to making this part of canonicalization IF the sorted 
> operand order produced by this utility is the canonical order of operands for 
> commutative operations, so that conflicts are not possible.

We can decide whatever we want the canonical ordering of operands to be for the 
Commutative trait. We don't have to leave things up to operations if it doesn't 
make sense.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124750

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

[PATCH] D123297: [flang][driver] Add support for -mmlir

[River Riddle via Phabricator via cfe-commits]

rriddle added inline comments.



Comment at: flang/test/Driver/mllvm_vs_mmlir.f90:17
+! MLLVM: flang (LLVM option parsing) [options]
+! MLLVM: --print-ir-after-all
+! MLLVM-NOT: --mlir-{{.*}}

awarzynski wrote:
> awarzynski wrote:
> > rovka wrote:
> > > rovka wrote:
> > > > Is this the most llvm-ish option we have? I'm concerned that MLIR might 
> > > > also decide to add an option that sounds like this (and for sure 
> > > > -print-ir-before-all is mentioned in the [[ 
> > > > https://mlir.llvm.org/getting_started/Debugging/ |  MLIR docs ]]).
> > > > Is this the most llvm-ish option we have? I'm concerned that MLIR might 
> > > > also decide to add an option that sounds like this (and for sure 
> > > > -print-ir-before-all is mentioned in the [[ 
> > > > https://mlir.llvm.org/getting_started/Debugging/ |  MLIR docs ]]).
> > > 
> > > Right, I think that might be why the premerge tests are failing...
> > > Is this the most llvm-ish option we have? 
> > 
> > Sadly, most LLVM options have rather generic names that could at some point 
> > be added in MLIR. Perhaps you'll spot something more suitable:
> > 
> > ```lang=bash
> > USAGE: flang (LLVM option parsing) [options]
> > 
> > OPTIONS:
> > 
> > Color Options:
> > 
> >   --color   - Use colors in output 
> > (default=autodetect)
> > 
> > General options:
> > 
> >   --aarch64-neon-syntax= - Choose style of NEON 
> > code to emit from AArch64 backend:
> > =generic-   Emit generic NEON 
> > assembly
> > =apple  -   Emit Apple-style 
> > NEON assembly
> >   --aarch64-use-aa  - Enable the use of AA 
> > during codegen.
> >   --abort-on-max-devirt-iterations-reached  - Abort when the max 
> > iterations for devirtualization CGSCC repeat pass is reached
> >   --allow-ginsert-as-artifact   - Allow G_INSERT to be 
> > considered an artifact. Hack around AMDGPU test infinite loops.
> >   --always-execute-loop-body- force the body of a 
> > loop to execute at least once
> >   --array-constructor-initial-buffer-size=- set the incremental 
> > array construction buffer size (default=32)
> >   --cfg-hide-cold-paths=- Hide blocks with 
> > relative frequency below the given value
> >   --cfg-hide-deoptimize-paths   -
> >   --cfg-hide-unreachable-paths  -
> >   --cost-kind=   - Target cost kind
> > =throughput -   Reciprocal 
> > throughput
> > =latency-   Instruction latency
> > =code-size  -   Code size
> > =size-latency   -   Code size and 
> > latency
> >   --debugify-level=  - Kind of debug info to 
> > add
> > =locations  -   Locations only
> > =location+variables -   Locations and 
> > Variables
> >   --debugify-quiet  - Suppress verbose 
> > debugify output
> >   --default-kinds= - string to set default 
> > kind values
> >   --disable-i2p-p2i-opt - Disables 
> > inttoptr/ptrtoint roundtrip optimization
> >   --dot-cfg-mssa= - file name for 
> > generated dot file
> >   --enable-cse-in-irtranslator  - Should enable CSE in 
> > irtranslator
> >   --enable-cse-in-legalizer - Should enable CSE in 
> > Legalizer
> >   --enable-gvn-memdep   -
> >   --enable-load-in-loop-pre -
> >   --enable-load-pre -
> >   --enable-loop-simplifycfg-term-folding-
> >   --enable-name-compression - Enable name/filename 
> > string compression
> >   --enable-split-backedge-in-load-pre   -
> >   --experimental-debug-variable-locations   - Use experimental new 
> > value-tracking variable locations
> >   --fdebug-dump-pre-fir - dump the Pre-FIR tree 
> > prior to FIR generation
> >   --fs-profile-debug-bw-threshold=- Only show debug 
> > message if the source branch weight is greater  than this value.
> >   --fs-profile-debug-prob-diff-threshold= - Only show debug 
> > message if the branch probility is greater than this value (in percentage).
> >   --gen-array-coor  - in lowering create 
> > ArrayCoorOp instead of CoordinateOp
> >   --generate-merged-base-profiles   - When generating 
> > nested context-sensitive profiles, always generate extra base profile for 
> > function with all its context profiles merged into it.
> >   --inline-all   

[PATCH] D113641: [llvm] Add a SFINAE template parameter to DenseMapInfo

[River Riddle via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG4c484f11d355: [llvm] Add a SFINAE template parameter to 
DenseMapInfo (authored by rriddle).

Changed prior to commit:
  https://reviews.llvm.org/D113641?vs=387386=387702#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113641

Files:
  clang/include/clang/AST/TypeOrdering.h
  clang/include/clang/Basic/SourceLocation.h
  clang/include/clang/Sema/Sema.h
  lldb/include/lldb/Utility/ConstString.h
  llvm/include/llvm/ADT/APInt.h
  llvm/include/llvm/ADT/APSInt.h
  llvm/include/llvm/ADT/ArrayRef.h
  llvm/include/llvm/ADT/DenseMapInfo.h
  llvm/include/llvm/ADT/Hashing.h
  llvm/include/llvm/ADT/ImmutableList.h
  llvm/include/llvm/ADT/PointerIntPair.h
  llvm/include/llvm/ADT/StringRef.h
  llvm/include/llvm/BinaryFormat/WasmTraits.h
  llvm/include/llvm/CodeGen/SelectionDAGNodes.h
  llvm/include/llvm/IR/Attributes.h
  llvm/include/llvm/Support/TypeSize.h
  llvm/lib/Support/APInt.cpp
  llvm/unittests/ADT/DenseMapTest.cpp
  mlir/include/mlir/Dialect/SPIRV/IR/SPIRVOps.h
  mlir/include/mlir/IR/Attributes.h
  mlir/include/mlir/IR/BuiltinOps.h
  mlir/include/mlir/IR/OpDefinition.h
  mlir/include/mlir/IR/Types.h
  mlir/include/mlir/Support/LLVM.h

Index: mlir/include/mlir/Support/LLVM.h
===
--- mlir/include/mlir/Support/LLVM.h
+++ mlir/include/mlir/Support/LLVM.h
@@ -46,7 +46,7 @@
 } // namespace detail
 template 
 class DenseMap;
-template  struct DenseMapInfo;
+template  struct DenseMapInfo;
 template  class DenseSet;
 class MallocAllocator;
 template  class MutableArrayRef;
@@ -90,7 +90,8 @@
 //
 // Containers.
 using llvm::ArrayRef;
-using llvm::DenseMapInfo;
+template 
+using DenseMapInfo = llvm::DenseMapInfo;
 template ,
   typename BucketT = llvm::detail::DenseMapPair>
Index: mlir/include/mlir/IR/Types.h
===
--- mlir/include/mlir/IR/Types.h
+++ mlir/include/mlir/IR/Types.h
@@ -269,6 +269,18 @@
   static unsigned getHashValue(mlir::Type val) { return mlir::hash_value(val); }
   static bool isEqual(mlir::Type LHS, mlir::Type RHS) { return LHS == RHS; }
 };
+template 
+struct DenseMapInfo::value>>
+: public DenseMapInfo {
+  static T getEmptyKey() {
+const void *pointer = llvm::DenseMapInfo::getEmptyKey();
+return T::getFromOpaquePointer(pointer);
+  }
+  static T getTombstoneKey() {
+const void *pointer = llvm::DenseMapInfo::getTombstoneKey();
+return T::getFromOpaquePointer(pointer);
+  }
+};
 
 /// We align TypeStorage by 8, so allow LLVM to steal the low bits.
 template <> struct PointerLikeTypeTraits {
Index: mlir/include/mlir/IR/OpDefinition.h
===
--- mlir/include/mlir/IR/OpDefinition.h
+++ mlir/include/mlir/IR/OpDefinition.h
@@ -1906,4 +1906,25 @@
 } // namespace impl
 } // end namespace mlir
 
+namespace llvm {
+
+template 
+struct DenseMapInfo<
+T, std::enable_if_t::value>> {
+  static inline T getEmptyKey() {
+auto *pointer = llvm::DenseMapInfo::getEmptyKey();
+return T::getFromOpaquePointer(pointer);
+  }
+  static inline T getTombstoneKey() {
+auto *pointer = llvm::DenseMapInfo::getTombstoneKey();
+return T::getFromOpaquePointer(pointer);
+  }
+  static unsigned getHashValue(T val) {
+return hash_value(val.getAsOpaquePointer());
+  }
+  static bool isEqual(T lhs, T rhs) { return lhs == rhs; }
+};
+
+} // end namespace llvm
+
 #endif
Index: mlir/include/mlir/IR/BuiltinOps.h
===
--- mlir/include/mlir/IR/BuiltinOps.h
+++ mlir/include/mlir/IR/BuiltinOps.h
@@ -49,23 +49,6 @@
 } // end namespace mlir
 
 namespace llvm {
-// Functions hash just like pointers.
-template <>
-struct DenseMapInfo {
-  static mlir::FuncOp getEmptyKey() {
-auto *pointer = llvm::DenseMapInfo::getEmptyKey();
-return mlir::FuncOp::getFromOpaquePointer(pointer);
-  }
-  static mlir::FuncOp getTombstoneKey() {
-auto *pointer = llvm::DenseMapInfo::getTombstoneKey();
-return mlir::FuncOp::getFromOpaquePointer(pointer);
-  }
-  static unsigned getHashValue(mlir::FuncOp val) {
-return hash_value(val.getAsOpaquePointer());
-  }
-  static bool isEqual(mlir::FuncOp lhs, mlir::FuncOp rhs) { return lhs == rhs; }
-};
-
 /// Allow stealing the low bits of FuncOp.
 template <>
 struct PointerLikeTypeTraits {
Index: mlir/include/mlir/IR/Attributes.h
===
--- mlir/include/mlir/IR/Attributes.h
+++ mlir/include/mlir/IR/Attributes.h
@@ -201,6 +201,19 @@
 return LHS == RHS;
   }
 };
+template 
+struct DenseMapInfo<
+T, std::enable_if_t::value>>
+: public DenseMapInfo {
+  static T getEmptyKey() {
+const void *pointer = 

[PATCH] D113641: [llvm] Add a SFINAE template parameter to DenseMapInfo

[River Riddle via Phabricator via cfe-commits]

rriddle updated this revision to Diff 387386.
rriddle added a comment.
Herald added subscribers: lldb-commits, hiraditya.
Herald added a project: LLDB.

resolve comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113641

Files:
  clang/include/clang/AST/TypeOrdering.h
  clang/include/clang/Basic/SourceLocation.h
  clang/include/clang/Sema/Sema.h
  lldb/include/lldb/Utility/ConstString.h
  llvm/include/llvm/ADT/APInt.h
  llvm/include/llvm/ADT/APSInt.h
  llvm/include/llvm/ADT/ArrayRef.h
  llvm/include/llvm/ADT/DenseMapInfo.h
  llvm/include/llvm/ADT/Hashing.h
  llvm/include/llvm/ADT/ImmutableList.h
  llvm/include/llvm/ADT/PointerIntPair.h
  llvm/include/llvm/ADT/StringRef.h
  llvm/include/llvm/BinaryFormat/WasmTraits.h
  llvm/include/llvm/CodeGen/SelectionDAGNodes.h
  llvm/include/llvm/IR/Attributes.h
  llvm/include/llvm/Support/TypeSize.h
  llvm/lib/Support/APInt.cpp
  llvm/unittests/ADT/DenseMapTest.cpp
  mlir/include/mlir/Dialect/SPIRV/IR/SPIRVOps.h
  mlir/include/mlir/IR/Attributes.h
  mlir/include/mlir/IR/BuiltinOps.h
  mlir/include/mlir/IR/OpDefinition.h
  mlir/include/mlir/IR/Types.h
  mlir/include/mlir/Support/LLVM.h

Index: mlir/include/mlir/Support/LLVM.h
===
--- mlir/include/mlir/Support/LLVM.h
+++ mlir/include/mlir/Support/LLVM.h
@@ -46,7 +46,8 @@
 } // namespace detail
 template 
 class DenseMap;
-template  struct DenseMapInfo;
+template 
+struct DenseMapInfo;
 template  class DenseSet;
 class MallocAllocator;
 template  class MutableArrayRef;
@@ -90,7 +91,8 @@
 //
 // Containers.
 using llvm::ArrayRef;
-using llvm::DenseMapInfo;
+template 
+using DenseMapInfo = llvm::DenseMapInfo;
 template ,
   typename BucketT = llvm::detail::DenseMapPair>
Index: mlir/include/mlir/IR/Types.h
===
--- mlir/include/mlir/IR/Types.h
+++ mlir/include/mlir/IR/Types.h
@@ -269,6 +269,18 @@
   static unsigned getHashValue(mlir::Type val) { return mlir::hash_value(val); }
   static bool isEqual(mlir::Type LHS, mlir::Type RHS) { return LHS == RHS; }
 };
+template 
+struct DenseMapInfo::value>>
+: public DenseMapInfo {
+  static T getEmptyKey() {
+const void *pointer = llvm::DenseMapInfo::getEmptyKey();
+return T::getFromOpaquePointer(pointer);
+  }
+  static T getTombstoneKey() {
+const void *pointer = llvm::DenseMapInfo::getTombstoneKey();
+return T::getFromOpaquePointer(pointer);
+  }
+};
 
 /// We align TypeStorage by 8, so allow LLVM to steal the low bits.
 template <> struct PointerLikeTypeTraits {
Index: mlir/include/mlir/IR/OpDefinition.h
===
--- mlir/include/mlir/IR/OpDefinition.h
+++ mlir/include/mlir/IR/OpDefinition.h
@@ -1906,4 +1906,25 @@
 } // namespace impl
 } // end namespace mlir
 
+namespace llvm {
+
+template 
+struct DenseMapInfo<
+T, std::enable_if_t::value>> {
+  static inline T getEmptyKey() {
+auto *pointer = llvm::DenseMapInfo::getEmptyKey();
+return T::getFromOpaquePointer(pointer);
+  }
+  static inline T getTombstoneKey() {
+auto *pointer = llvm::DenseMapInfo::getTombstoneKey();
+return T::getFromOpaquePointer(pointer);
+  }
+  static unsigned getHashValue(T val) {
+return hash_value(val.getAsOpaquePointer());
+  }
+  static bool isEqual(T lhs, T rhs) { return lhs == rhs; }
+};
+
+} // end namespace llvm
+
 #endif
Index: mlir/include/mlir/IR/BuiltinOps.h
===
--- mlir/include/mlir/IR/BuiltinOps.h
+++ mlir/include/mlir/IR/BuiltinOps.h
@@ -49,23 +49,6 @@
 } // end namespace mlir
 
 namespace llvm {
-// Functions hash just like pointers.
-template <>
-struct DenseMapInfo {
-  static mlir::FuncOp getEmptyKey() {
-auto *pointer = llvm::DenseMapInfo::getEmptyKey();
-return mlir::FuncOp::getFromOpaquePointer(pointer);
-  }
-  static mlir::FuncOp getTombstoneKey() {
-auto *pointer = llvm::DenseMapInfo::getTombstoneKey();
-return mlir::FuncOp::getFromOpaquePointer(pointer);
-  }
-  static unsigned getHashValue(mlir::FuncOp val) {
-return hash_value(val.getAsOpaquePointer());
-  }
-  static bool isEqual(mlir::FuncOp lhs, mlir::FuncOp rhs) { return lhs == rhs; }
-};
-
 /// Allow stealing the low bits of FuncOp.
 template <>
 struct PointerLikeTypeTraits {
Index: mlir/include/mlir/IR/Attributes.h
===
--- mlir/include/mlir/IR/Attributes.h
+++ mlir/include/mlir/IR/Attributes.h
@@ -201,6 +201,19 @@
 return LHS == RHS;
   }
 };
+template 
+struct DenseMapInfo<
+T, std::enable_if_t::value>>
+: public DenseMapInfo {
+  static T getEmptyKey() {
+const void *pointer = llvm::DenseMapInfo::getEmptyKey();
+return T::getFromOpaquePointer(pointer);
+  }
+  static T getTombstoneKey() {
+const void *pointer = 

[PATCH] D113641: [llvm] Add a SFINAE template parameter to DenseMapInfo

[River Riddle via Phabricator via cfe-commits]

rriddle added inline comments.



Comment at: llvm/include/llvm/ADT/Hashing.h:59
 namespace llvm {
-template  struct DenseMapInfo;
 

lattner wrote:
> Is there a way to keep the forward declarations references here instead of 
> #include?  DenseMapInfo.h pulls in a ton of stuff including  and 
> 
I mentioned it in a comment. If we don't want the includes, we'll need to 
sprinkle `void` everywhere that we specialize the template. (I don't have a 
preference either way)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113641

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

[PATCH] D113641: [llvm] Add a SFINAE template parameter to DenseMapInfo

[River Riddle via Phabricator via cfe-commits]

rriddle added a comment.

Given the new template parameter, I needed to update the forward declarations. 
Some of them already had DenseMapInfo from an include, so I just dropped them. 
Some didn't, so I opted to just add an include for DenseMapInfo (seemed small 
enough). I could avoid the includes by adding an explicit `void` for the second 
template parameter. Let me know if there is a preference.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113641

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

[PATCH] D113641: [llvm] Add a SFINAE template parameter to DenseMapInfo

[River Riddle via Phabricator via cfe-commits]

rriddle created this revision.
rriddle added reviewers: dblaikie, mehdi_amini, lattner, silvas.
Herald added subscribers: wenzhicui, wrengr, Chia-hungDuan, dcaballe, cota, 
teijeong, dexonsmith, rdzhabarov, tatianashp, ThomasRaoux, jdoerfert, 
AlexeySotkin, msifontes, jurahul, Kayjukh, grosul1, Joonsoo, liufengdb, 
aartbik, lucyrfox, mgester, arpith-jacob, antiagainst, shauheen, mravishankar, 
bollu, sbc100.
Herald added a reviewer: antiagainst.
Herald added a reviewer: ftynse.
rriddle requested review of this revision.
Herald added subscribers: llvm-commits, cfe-commits, stephenneuendorffer, 
nicolasvasilache, aheejin.
Herald added projects: clang, MLIR, LLVM.

This allows for using SFINAE partial specialization for DenseMapInfo.
In MLIR, this is particularly useful as it will allow for defining partial
specializations that support all Attribute, Op, and Type classes without
needing to specialize DenseMapInfo for each individual class.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D113641

Files:
  clang/include/clang/AST/TypeOrdering.h
  clang/include/clang/Basic/SourceLocation.h
  clang/include/clang/Sema/Sema.h
  llvm/include/llvm/ADT/APInt.h
  llvm/include/llvm/ADT/ArrayRef.h
  llvm/include/llvm/ADT/DenseMapInfo.h
  llvm/include/llvm/ADT/Hashing.h
  llvm/include/llvm/ADT/ImmutableList.h
  llvm/include/llvm/ADT/PointerIntPair.h
  llvm/include/llvm/ADT/StringRef.h
  llvm/include/llvm/BinaryFormat/WasmTraits.h
  llvm/include/llvm/CodeGen/SelectionDAGNodes.h
  llvm/include/llvm/IR/Attributes.h
  llvm/include/llvm/Support/TypeSize.h
  llvm/unittests/ADT/DenseMapTest.cpp
  mlir/include/mlir/Dialect/SPIRV/IR/SPIRVOps.h
  mlir/include/mlir/IR/Attributes.h
  mlir/include/mlir/IR/BuiltinOps.h
  mlir/include/mlir/IR/OpDefinition.h
  mlir/include/mlir/IR/Types.h
  mlir/include/mlir/Support/LLVM.h

Index: mlir/include/mlir/Support/LLVM.h
===
--- mlir/include/mlir/Support/LLVM.h
+++ mlir/include/mlir/Support/LLVM.h
@@ -46,7 +46,8 @@
 } // namespace detail
 template 
 class DenseMap;
-template  struct DenseMapInfo;
+template 
+struct DenseMapInfo;
 template  class DenseSet;
 class MallocAllocator;
 template  class MutableArrayRef;
@@ -90,7 +91,8 @@
 //
 // Containers.
 using llvm::ArrayRef;
-using llvm::DenseMapInfo;
+template 
+using DenseMapInfo = llvm::DenseMapInfo;
 template ,
   typename BucketT = llvm::detail::DenseMapPair>
Index: mlir/include/mlir/IR/Types.h
===
--- mlir/include/mlir/IR/Types.h
+++ mlir/include/mlir/IR/Types.h
@@ -269,6 +269,18 @@
   static unsigned getHashValue(mlir::Type val) { return mlir::hash_value(val); }
   static bool isEqual(mlir::Type LHS, mlir::Type RHS) { return LHS == RHS; }
 };
+template 
+struct DenseMapInfo::value>>
+: public DenseMapInfo {
+  static T getEmptyKey() {
+const void *pointer = llvm::DenseMapInfo::getEmptyKey();
+return T::getFromOpaquePointer(pointer);
+  }
+  static T getTombstoneKey() {
+const void *pointer = llvm::DenseMapInfo::getTombstoneKey();
+return T::getFromOpaquePointer(pointer);
+  }
+};
 
 /// We align TypeStorage by 8, so allow LLVM to steal the low bits.
 template <> struct PointerLikeTypeTraits {
Index: mlir/include/mlir/IR/OpDefinition.h
===
--- mlir/include/mlir/IR/OpDefinition.h
+++ mlir/include/mlir/IR/OpDefinition.h
@@ -1906,4 +1906,25 @@
 } // namespace impl
 } // end namespace mlir
 
+namespace llvm {
+
+template 
+struct DenseMapInfo<
+T, std::enable_if_t::value>> {
+  static inline T getEmptyKey() {
+auto *pointer = llvm::DenseMapInfo::getEmptyKey();
+return T::getFromOpaquePointer(pointer);
+  }
+  static inline T getTombstoneKey() {
+auto *pointer = llvm::DenseMapInfo::getTombstoneKey();
+return T::getFromOpaquePointer(pointer);
+  }
+  static unsigned getHashValue(T val) {
+return hash_value(val.getAsOpaquePointer());
+  }
+  static bool isEqual(T lhs, T rhs) { return lhs == rhs; }
+};
+
+} // end namespace llvm
+
 #endif
Index: mlir/include/mlir/IR/BuiltinOps.h
===
--- mlir/include/mlir/IR/BuiltinOps.h
+++ mlir/include/mlir/IR/BuiltinOps.h
@@ -49,23 +49,6 @@
 } // end namespace mlir
 
 namespace llvm {
-// Functions hash just like pointers.
-template <>
-struct DenseMapInfo {
-  static mlir::FuncOp getEmptyKey() {
-auto *pointer = llvm::DenseMapInfo::getEmptyKey();
-return mlir::FuncOp::getFromOpaquePointer(pointer);
-  }
-  static mlir::FuncOp getTombstoneKey() {
-auto *pointer = llvm::DenseMapInfo::getTombstoneKey();
-return mlir::FuncOp::getFromOpaquePointer(pointer);
-  }
-  static unsigned getHashValue(mlir::FuncOp val) {
-return hash_value(val.getAsOpaquePointer());
-  }
-  static bool isEqual(mlir::FuncOp lhs, mlir::FuncOp rhs) { return lhs == rhs; }
-};
-
 /// Allow stealing the low 

[PATCH] D83088: Introduce CfgTraits abstraction

[River Riddle via Phabricator via cfe-commits]

rriddle added inline comments.



Comment at: mlir/include/mlir/IR/Dominance.h:49
+template <>
+struct llvm::CfgTraitsFor {
+  using CfgTraits = mlir::CfgTraits;

This seems to have broken the GCC5 build:
https://buildkite.com/mlir/mlir-core/builds/8739#7a957564-9850-487c-a814-c6818890bd14

```
/mlir/include/mlir/IR/Dominance.h:49:14: error: specialization of 
'template struct llvm::CfgTraitsFor' in different 
namespace [-fpermissive]
 struct llvm::CfgTraitsFor {
  ^
In file included from mlir/include/mlir/IR/Dominance.h:13:0,
 from mlir/lib/IR/Verifier.cpp:30:
llvm/include/llvm/Support/CfgTraits.h:294:44: error:   from definition of 
'template struct llvm::CfgTraitsFor' [-fpermissive]
 template  struct CfgTraitsFor;
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83088

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

[PATCH] D83087: DomTree: remove explicit use of DomTreeNodeBase::iterator

[River Riddle via Phabricator via cfe-commits]

rriddle accepted this revision.
rriddle added a comment.

Approval for anything MLIR related.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83087



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

[PATCH] D81045: [LLVM] Change isa<> to a variadic function template

[River Riddle via Phabricator via cfe-commits]

rriddle accepted this revision.
rriddle marked an inline comment as done.
rriddle added a comment.
This revision is now accepted and ready to land.

I have run into the desire to have this on many occasions, so LGTM for me.




Comment at: clang/include/clang/AST/DeclBase.h:521
 AttrVec  = getAttrs();
-Vec.erase(std::remove_if(Vec.begin(), Vec.end(), isa), 
Vec.end());
+Vec.erase(std::remove_if(Vec.begin(), Vec.end(),
+ [](Attr *A) { return isa(A); }),

nit: We could also update this to use `llvm::erase_if`, but fine to leave as is.



Comment at: llvm/include/llvm/Support/Casting.h:147
+template 
+LLVM_NODISCARD inline typename std::enable_if::type
+isa(const Y ) {

nit: I would remove the enable_if and just add an additional type to the 
template.

```
template
... isa(const Y ) {
  return isa(Val) || isa(Val);
}
```



Comment at: llvm/include/llvm/Support/Casting.h:136
+// isa - Return true if the parameter to the template is an instance of one
+// of the template type argument.  Used like this:
 //

nit: arguments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81045



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

[PATCH] D71775: [ThreadPool] On Windows, extend usage to all CPU sockets and all NUMA groups

[River Riddle via Phabricator via cfe-commits]

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

(Appeasing herald for MLIR changes).


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

https://reviews.llvm.org/D71775



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

[PATCH] D73437: [mlir][spirv] Convert linalg.generic for reduction to SPIR-V ops

[River Riddle via Phabricator via cfe-commits]

rriddle added inline comments.



Comment at: mlir/include/mlir/Dialect/SPIRV/SPIRVControlFlowOps.td:463
+Location loc, Value condition,
+llvm::function_ref thenBody,
+OpBuilder *builder);

Drop the llvm::.



Comment at: mlir/lib/Conversion/LinalgToSPIRV/LinalgToSPIRV.cpp:32
+
+  return llvm::all_of(op->getOperands(), isOfMemrefType) &&
+ llvm::all_of(op->getResults(), isOfMemrefType);

nit: You can use getOperandTypes() && getResultTypes() respectively.



Comment at: mlir/lib/Conversion/LinalgToSPIRV/LinalgToSPIRV.cpp:38
+static bool isLinalgSingleReductionIterator(ArrayAttr iterators) {
+  if (iterators.getValue().size() != 1)
+return false;

Can you use size() directly on the ArrayAttr?



Comment at: mlir/lib/Conversion/LinalgToSPIRV/LinalgToSPIRV.cpp:66
+};
+}
+

nit: Missing end namespace comment.



Comment at: mlir/lib/Conversion/LinalgToSPIRV/LinalgToSPIRV.cpp:80
+  auto  = op.region();
+  if (region.empty() || !has_single_element(region.getBlocks()))
+return BinaryOpKind::Unknown;

You should be able to remove the .empty() call as well as the .getBlocks()


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73437



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