[PATCH] D73713: Fixed non-deterministic GPU intrisnic lowering.

[Julian Gross via Phabricator via llvm-commits]

dfki-jugr created this revision.
Herald added subscribers: llvm-commits, liufengdb, herhut, lucyrfox, mgester, arpith-jacob, csigg, antiagainst, shauheen, burmako, jpienaar, rriddle, mehdi_amini, dmgreen, mgrang, jholewinski.
Herald added a reviewer: nicolasvasilache.
Herald added a reviewer: rriddle.
Herald added a project: LLVM.

The current implementation of the intrinsic lowering phases seemed to be
non-determinsitic accross platforms. Multiple patterns with the same
priority have been matched differently on different platforms (Windows/
Linux) (see https://reviews.llvm.org/D73471). This patch circumvents
this issue by using an adjusted pattern-rewriter benefit to avoid
clashes.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D73713

Files:
  mlir/include/mlir/Conversion/StandardToLLVM/ConvertStandardToLLVM.h
  mlir/include/mlir/IR/PatternMatch.h
  mlir/lib/Conversion/GPUCommon/OpToFuncCallLowering.h
  mlir/lib/Conversion/GPUToNVVM/LowerGpuOpsToNVVMOps.cpp
  mlir/lib/Conversion/GPUToROCDL/LowerGpuOpsToROCDLOps.cpp
  mlir/lib/IR/PatternMatch.cpp


Index: mlir/lib/IR/PatternMatch.cpp
===================================================================
--- mlir/lib/IR/PatternMatch.cpp
+++ mlir/lib/IR/PatternMatch.cpp
@@ -24,6 +24,12 @@
   return representation;
 }
 
+PatternBenefit PatternBenefit::increase() const {
+  if (representation + 1 < ImpossibleToMatchSentinel)
+    return PatternBenefit(representation + 1);
+  return *this;
+}
+
 //===----------------------------------------------------------------------===//
 // Pattern implementation
 //===----------------------------------------------------------------------===//
Index: mlir/lib/Conversion/GPUToROCDL/LowerGpuOpsToROCDLOps.cpp
===================================================================
--- mlir/lib/Conversion/GPUToROCDL/LowerGpuOpsToROCDLOps.cpp
+++ mlir/lib/Conversion/GPUToROCDL/LowerGpuOpsToROCDLOps.cpp
@@ -58,8 +58,9 @@
                                                  "__ocml_cos_f64");
     patterns.insert<OpToFuncCallLowering<ExpOp>>(converter, "__ocml_exp_f32",
                                                  "__ocml_exp_f64");
-    patterns.insert<OpToFuncCallLowering<TanhOp>>(converter, "__ocml_tanh_f32",
-                                                  "__ocml_tanh_f64");
+    patterns.insert<OpToFuncCallLowering<TanhOp>>(
+        converter, "__ocml_tanh_f32", "__ocml_tanh_f64",
+        LLVMOpLowering::getDefaultBenefit().increase());
 
     ConversionTarget target(getContext());
     target.addLegalDialect<LLVM::LLVMDialect, ROCDL::ROCDLDialect>();
Index: mlir/lib/Conversion/GPUToNVVM/LowerGpuOpsToNVVMOps.cpp
===================================================================
--- mlir/lib/Conversion/GPUToNVVM/LowerGpuOpsToNVVMOps.cpp
+++ mlir/lib/Conversion/GPUToNVVM/LowerGpuOpsToNVVMOps.cpp
@@ -746,8 +746,9 @@
                                                "__nv_cos");
   patterns.insert<OpToFuncCallLowering<ExpOp>>(converter, "__nv_expf",
                                                "__nv_exp");
-  patterns.insert<OpToFuncCallLowering<TanhOp>>(converter, "__nv_tanhf",
-                                                "__nv_tanh");
+  patterns.insert<OpToFuncCallLowering<TanhOp>>(
+      converter, "__nv_tanhf", "__nv_tanh",
+      LLVMOpLowering::getDefaultBenefit().increase());
 }
 
 std::unique_ptr<OpPassBase<gpu::GPUModuleOp>>
Index: mlir/lib/Conversion/GPUCommon/OpToFuncCallLowering.h
===================================================================
--- mlir/lib/Conversion/GPUCommon/OpToFuncCallLowering.h
+++ mlir/lib/Conversion/GPUCommon/OpToFuncCallLowering.h
@@ -29,9 +29,10 @@
 struct OpToFuncCallLowering : public LLVMOpLowering {
 public:
   explicit OpToFuncCallLowering(LLVMTypeConverter &lowering_, StringRef f32Func,
-                                StringRef f64Func)
+                                StringRef f64Func,
+                                PatternBenefit benefit = getDefaultBenefit())
       : LLVMOpLowering(SourceOp::getOperationName(),
-                       lowering_.getDialect()->getContext(), lowering_),
+                       lowering_.getDialect()->getContext(), lowering_, benefit),
         f32Func(f32Func), f64Func(f64Func) {}
 
   PatternMatchResult
Index: mlir/include/mlir/IR/PatternMatch.h
===================================================================
--- mlir/include/mlir/IR/PatternMatch.h
+++ mlir/include/mlir/IR/PatternMatch.h
@@ -41,6 +41,9 @@
   // corresponding pattern isImpossibleToMatch() then this aborts.
   unsigned short getBenefit() const;
 
+  /// Increases the current benefit by one (if possible).
+  PatternBenefit increase() const;
+
   bool operator==(const PatternBenefit &rhs) const {
     return representation == rhs.representation;
   }
Index: mlir/include/mlir/Conversion/StandardToLLVM/ConvertStandardToLLVM.h
===================================================================
--- mlir/include/mlir/Conversion/StandardToLLVM/ConvertStandardToLLVM.h
+++ mlir/include/mlir/Conversion/StandardToLLVM/ConvertStandardToLLVM.h
@@ -230,8 +230,12 @@
 /// purpose of type conversions.
 class LLVMOpLowering : public ConversionPattern {
 public:
+  /// Returns the default benefit value of all LLVM op lowering patterns.
+  static PatternBenefit getDefaultBenefit() { return PatternBenefit(1); }
+
   LLVMOpLowering(StringRef rootOpName, MLIRContext *context,
-                 LLVMTypeConverter &lowering, PatternBenefit benefit = 1);
+                 LLVMTypeConverter &lowering,
+                 PatternBenefit benefit = getDefaultBenefit());
 
 protected:
   // Back-reference to the lowering class, used to call type and function


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D73713.241456.patch
Type: text/x-patch
Size: 4603 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200130/4e4b76a7/attachment-0001.bin>