[polly] r292213 - Relax assert when setting access functions with invariant base pointers

[Friedman, Eli via llvm-commits]

On 1/17/2017 4:00 AM, Tobias Grosser via llvm-commits wrote:
> Author: grosser
> Date: Tue Jan 17 06:00:42 2017
> New Revision: 292213
>
> URL: http://llvm.org/viewvc/llvm-project?rev=292213&view=rev
> Log:
> Relax assert when setting access functions with invariant base pointers
>
> Summary:
> Instead of forbidding such access functions completely, we verify that their
> base pointer has been hoisted and only assert in case the base pointer was
> not hoisted.
>
> I was trying for a little while to get a test case that ensures the assert is
> correctly fired in case of invariant load hoisting being disabled, but I could
> not find a good way to do so, as llvm-lit immediately aborts if a command
> yields a non-zero return value. As we do not generally test our asserts,
> not having a test case here seems OK.
>
> This resolves http://llvm.org/PR31494
>
> Suggested-by: Michael Kruse <llvm at meinersbur.de>
>
> Reviewers: efriedma, jdoerfert, Meinersbur, gareevroman, sebpop, zinob, huihuiz, pollydev
>
> Reviewed By: Meinersbur
>
> Differential Revision: https://reviews.llvm.org/D28798
>
> Modified:
>      polly/trunk/lib/Analysis/ScopInfo.cpp
>      polly/trunk/lib/CodeGen/IslNodeBuilder.cpp
>      polly/trunk/test/Isl/CodeGen/MemAccess/invariant_base_ptr.ll
>
> Modified: polly/trunk/lib/Analysis/ScopInfo.cpp
> URL: http://llvm.org/viewvc/llvm-project/polly/trunk/lib/Analysis/ScopInfo.cpp?rev=292213&r1=292212&r2=292213&view=diff
> ==============================================================================
> --- polly/trunk/lib/Analysis/ScopInfo.cpp (original)
> +++ polly/trunk/lib/Analysis/ScopInfo.cpp Tue Jan 17 06:00:42 2017
> @@ -1078,16 +1078,24 @@ void MemoryAccess::setNewAccessRelation(
>     isl_set_free(NewDomain);
>     isl_set_free(StmtDomain);
>   
> -  // Check whether access dimensions correspond to number of dimensions of the
> -  // accesses array.
>     auto *NewAccessSpace = isl_space_range(NewSpace);
>     assert(isl_space_has_tuple_id(NewAccessSpace, isl_dim_set) &&
>            "Must specify the array that is accessed");
>     auto *NewArrayId = isl_space_get_tuple_id(NewAccessSpace, isl_dim_set);
>     auto *SAI = static_cast<ScopArrayInfo *>(isl_id_get_user(NewArrayId));
>     assert(SAI && "Must set a ScopArrayInfo");
> -  assert(!SAI->getBasePtrOriginSAI() &&
> -         "Indirect array not supported by codegen");
> +
> +  if (SAI->isArrayKind() && SAI->getBasePtrOriginSAI()) {
> +    InvariantEquivClassTy *EqClass =
> +        getStatement()->getParent()->lookupInvariantEquivClass(
> +            SAI->getBasePtr());
> +    assert(EqClass &&
> +           "Access functions to indirect arrays must have an invariant and "
> +           "hoisted base pointer");
> +  }
> +

This is going to produce an "unused variable EqClass" warning in builds 
with assertions turned off; maybe put the whole if statement into an 
"#ifndef NDEBUG"?

-Eli

-- 
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project