Page MenuHomePhabricator

[Polly] [PPCGCodeGeneration] Handle 1-d pointers to scalars in array size.
AbandonedPublic

Authored by bollu on Jul 21 2017, 5:35 AM.

Details

Summary
  • We try to multiply "offset"s together.
  • Case when array is 1D and offset is 0 is not handled.
  • Detect this case and return the size of the underlying object.
  • Feels like a hack. @grosser: This is a hack, right?

Event Timeline

bollu created this revision.Jul 21 2017, 5:35 AM
bollu retitled this revision from [PPCGCodeGeneration] Handle 1-d pointers to scalars in array size. to [Polly] [PPCGCodeGeneration] Handle 1-d pointers to scalars in array size..Jul 21 2017, 6:10 AM
grosser edited edge metadata.Jul 21 2017, 11:13 AM

Yes, this is a total hack. The underlying bugs are:

  1. Scop::getAccessesOfType does not collect access relations on invariant accesses (which causes us to derive an empty "extent"

If we fix this, I expect this test case to work.

Now, in general we can likely not avoid zero size allocations (in rare corner cases). We may at some point allow them, but for now it is a good assertion that fires in case of such situations. We should see if Sanjay's problems are also resolved after fixing 1).

lib/CodeGen/PPCGCodeGeneration.cpp
740

Unrelated.

grosser requested changes to this revision.Jul 22 2017, 2:02 PM

I mark this as requestion changes. Siddharth, can you check if my comment leads to a cleaner solution?

This revision now requires changes to proceed.Jul 22 2017, 2:02 PM
bollu added a comment.Jul 23 2017, 7:17 AM

@grosser I'm not sure how to build up the correct access union map.

incorrect-version-of-get-accesses.cpp
__isl_give isl_union_map *
Scop::getAccessesOfType(std::function<bool(MemoryAccess &)> Predicate) {
  isl_union_map *Accesses = isl_union_map_empty(getParamSpace());

  const auto shouldConsiderInvEquivClass =
      [&Predicate](const InvariantEquivClassTy &InvEquivClass) -> bool {
    for (MemoryAccess *MA : InvEquivClass.InvariantAccesses) {
      if (!Predicate(*MA))
        return false;
    }
    return true;
  };

  for (InvariantEquivClassTy &InvEquivClass : getInvariantAccesses()) {
    if (!shouldConsiderInvEquivClass(InvEquivClass))
      continue;

    for (MemoryAccess *MA : InvEquivClass.InvariantAccesses) {
      isl_map *AccessDomain = MA->getAccessRelation().release();
      // This is incorrect, naturally. We don't limit this access relation's domain.
      Accesses = isl_union_map_add_map(Accesses, AccessDomain);
    }
  }

  for (ScopStmt &Stmt : *this) {
    for (MemoryAccess *MA : Stmt) {
      if (!Predicate(*MA))
        continue;

      isl_set *Domain = Stmt.getDomain();
      isl_map *AccessDomain = MA->getAccessRelation().release();
      AccessDomain = isl_map_intersect_domain(AccessDomain, Domain);
      Accesses = isl_union_map_add_map(Accesses, AccessDomain);
    }
  }
  return isl_union_map_coalesce(Accesses);
}

However, the InvariantEquivClassTy doesn't seem to hold information on a per-memory access basis as to what the valid context is.
So, I'm not sure what to do.

singam-sanjay edited edge metadata.Jul 23 2017, 8:34 AM

@grosser How does solving 1. relate to D35630 ?

@bollu Does this patch help codegenerate cases where the SCOP's constants are essentially scalars but are considered arrays with zero elements ?

@grosser How does solving 1. relate to D35630 ?

Not at all. I was referring to https://bugs.llvm.org/show_bug.cgi?id=33324

@bollu Does this patch help codegenerate cases where the SCOP's constants are essentially scalars but are considered arrays with zero elements ?

bollu abandoned this revision.Jul 24 2017, 5:42 AM

Abandoned in favour of D35795 which actually solves the problem.