[PATCH] D45066: [Polly] [ScopInfo] Remove bail out condition in buildMinMaxAccess()

[Michael Kruse via Phabricator via llvm-commits]

Meinersbur added inline comments.


================
Comment at: lib/Analysis/ScopInfo.cpp:2351-2352
   // allocated array but we will never dereference it anyway.
-  assert(MaxPMA.dim(isl::dim::out) && "Assumed at least one output dimension");
+  assert(((!MaxPMA || !MinPMA) || MaxPMA.dim(isl::dim::out)) &&
+         "Assumed at least one output dimension");
+
----------------
cs15btech11044 wrote:
> Meinersbur wrote:
> > You should decide between one of the following (not a mix of both):
> > 1. Check for an error and return before reaching the assert
> > 2. Let the assertion pass in case of `MaxPMA` being NULL.
> > 
> > Please tell me if you are unsure what to do.
> > 
> I am a bit unsure about how to proceed.
> 
> I am thinking of checking `MaxPMA` and `MinPMA` before assertion and returning error if either of them are `NULL`, along with removing these checks from the assertion.
> 
> Would that be suitable?
This is the first suggestion (which is what I think you just described):
```
  MinPMA = Set.lexmin_pw_multi_aff();
  MaxPMA = Set.lexmax_pw_multi_aff();

  MinPMA = MinPMA.coalesce();
  MaxPMA = MaxPMA.coalesce();

  // Adjust the last dimension of the maximal access by one as we want to
  // enclose the accessed memory region by MinPMA and MaxPMA. The pointer
  // we test during code generation might now point after the end of the
  // allocated array but we will never dereference it anyway.
  if (!MinPMA || !MaxPMA)
    return isl::stat::error;
  assert(MaxPMA.dim(isl::dim::out) && "Assumed at least one output dimension");
  Pos = MaxPMA.dim(isl::dim::out) - 1;
  LastDimAff = MaxPMA.get_pw_aff(Pos);
  OneAff = isl::aff(isl::local_space(LastDimAff.get_domain_space()));
  OneAff = OneAff.add_constant_si(1);
  LastDimAff = LastDimAff.add(OneAff);
  MaxPMA = MaxPMA.set_pw_aff(Pos, LastDimAff);

  if (!MinPMA || !MaxPMA)
    return isl::stat::error;
  MinMaxAccesses.push_back(std::make_pair(MinPMA, MaxPMA));

  return isl::stat::ok;
```
(Also checking `MinPMA` before the assertion is not strictly necessary)


This is the second suggestion:
```
  MinPMA = Set.lexmin_pw_multi_aff();
  MaxPMA = Set.lexmax_pw_multi_aff();

  MinPMA = MinPMA.coalesce();
  MaxPMA = MaxPMA.coalesce();

  // Adjust the last dimension of the maximal access by one as we want to
  // enclose the accessed memory region by MinPMA and MaxPMA. The pointer
  // we test during code generation might now point after the end of the
  // allocated array but we will never dereference it anyway.
  assert((!MaxPMA || MaxPMA.dim(isl::dim::out)) && "Assumed at least one output dimension");
  Pos = MaxPMA.dim(isl::dim::out) - 1;
  LastDimAff = MaxPMA.get_pw_aff(Pos);
  OneAff = isl::aff(isl::local_space(LastDimAff.get_domain_space()));
  OneAff = OneAff.add_constant_si(1);
  LastDimAff = LastDimAff.add(OneAff);
  MaxPMA = MaxPMA.set_pw_aff(Pos, LastDimAff);

  if (!MinPMA || !MaxPMA)
    return isl::stat::error;
  MinMaxAccesses.push_back(std::make_pair(MinPMA, MaxPMA));

  return isl::stat::ok;
```



Repository:
  rPLO Polly

https://reviews.llvm.org/D45066