[BlockGenerator] Take context into account when identifying partial writes

[grosser]

Summary

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 <[e-mail]>

[Meinersbur]

LGTM

You forgot to add [Polly] to the patch title (Please don't add it to the commit).

[grosser]

Thanks, the [Polly], indeed.

lib/CodeGen/BlockGenerators.cpp

void BlockGenerator::generateConditionalExecution(
 
   // Don't call GenThenFunc if it is never executed. An ast index expression
   // might not be defined in this case.
   if (Subdomain.is_empty())
     return;
 
   // 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;
   }
 
   // Generate the condition.
   Value *Cond = buildContainsCondition(Stmt, Subdomain);
   BasicBlock *HeadBlock = Builder.GetInsertBlock();
   StringRef BlockName = HeadBlock->getName();

test/Isl/CodeGen/partial_write_full_write_that_appears_partial.ll

+; 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
+}