[PATCH] D33759: [BlockGenerator] Take context into account when identifying partial writes

[Tobias Grosser via Phabricator via llvm-commits]

grosser created this revision.
grosser added a project: Polly.
Herald added a subscriber: srhines.

A partial write is a write where the domain of the values written is a subset of
the execution domain of the parent statement containing the write. Originally,
we directly checked this subset relation whereas it is indeed only important
that the subset relation holds for the parameter values that are known to be
valid in the execution context of the scop. We update our check to avoid the
unnecessary introduction of partial writes in situations where the write appears
to be partial without context information, but where context information allows
us to understand that a full write can be generated.

This change fixes (hides) a recent regression introduced in r303517, which broke
our AOSP builds. The part that is correctly fixed in this change is that we do
not any more unnecessarily generate a partial write. This is good performance
wise and, as we currently do not yet explicitly introduce partial writes in the
default configuration, this also hides possible bugs in the partial writes
implementation. The crashes that we have originally seen were caused by such
a bug, where partial writes were incorrectly generated in region statements. An
additional patch in a subsequent commit is needed to address this problem.

Reported-by: Eli Friedman <efriedma at codeaurora.org>


https://reviews.llvm.org/D33759

Files:
  lib/CodeGen/BlockGenerators.cpp
  test/Isl/CodeGen/partial_write_full_write_that_appears_partial.ll


Index: test/Isl/CodeGen/partial_write_full_write_that_appears_partial.ll
===================================================================
--- /dev/null
+++ test/Isl/CodeGen/partial_write_full_write_that_appears_partial.ll
@@ -0,0 +1,34 @@
+; RUN: opt %loadPolly -polly-codegen -S < %s | FileCheck %s
+
+; CHECK:      polly.stmt.if.then81:                             ; preds = %polly.stmt.if.end75
+; CHECK-NEXT:   %scevgep = getelementptr [2 x %S], [2 x %S]* %tmp, i64 0, i64 %.147
+; CHECK-NEXT:   %scevgep1 = bitcast %S* %scevgep to float*
+; CHECK-NEXT:   store float undef, float* %scevgep1, align 4, !alias.scope !0, !noalias !2
+; CHECK-NEXT:   br label %polly.stmt.if.end87.region_exiting
+
+target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
+target triple = "aarch64--linux-android"
+
+%S = type { float, float }
+
+define void @f() {
+entry:
+  %tmp = alloca [2 x %S], align 4
+  %cmp52 = fcmp olt float undef, undef
+  %not.cmp52 = xor i1 %cmp52, true
+  %.147 = zext i1 %not.cmp52 to i64
+  %fX64 = getelementptr inbounds [2 x %S], [2 x %S]* %tmp, i64 0, i64 %.147, i32 0
+  br label %if.end75
+
+if.end75:
+  %cmp80 = fcmp olt float undef, undef
+  br i1 %cmp80, label %if.then81, label %if.end87
+
+if.then81:
+  store float undef, float* %fX64, align 4
+  br label %if.end87
+
+if.end87:
+  %0 = phi float [ undef, %if.then81 ], [ undef, %if.end75 ]
+  ret void
+}
Index: lib/CodeGen/BlockGenerators.cpp
===================================================================
--- lib/CodeGen/BlockGenerators.cpp
+++ lib/CodeGen/BlockGenerators.cpp
@@ -595,7 +595,10 @@
 
   // If the condition is a tautology, don't generate a condition around the
   // code.
-  if (StmtDom.is_subset(Subdomain)) {
+  bool IsPartialWrite =
+      !StmtDom.intersect_params(give(Stmt.getParent()->getContext()))
+           .is_subset(Subdomain);
+  if (!IsPartialWrite) {
     GenThenFunc();
     return;
   }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D33759.100965.patch
Type: text/x-patch
Size: 1932 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170601/8db3fcf6/attachment.bin>