[llvm-commits] [polly] r143576 - /polly/trunk/lib/Analysis/ScopDetection.cpp

[Sebastian Pop]

On Wed, Nov 2, 2011 at 4:40 PM, Tobias Grosser
<grosser at fim.uni-passau.de> wrote:
> Author: grosser
> Date: Wed Nov  2 16:40:08 2011
> New Revision: 143576
>
> URL: http://llvm.org/viewvc/llvm-project?rev=143576&view=rev
> Log:
> ScopDetection: Add new SCEV Validator
>
> The SCEV Validator is used to check if the bound of a loop can be translated
> into a polyhedral constraint. The new validator is more general as the check
> used previously and e.g. allows bounds like 'smax 1, %a'. At the moment, we
> only allow signed comparisons. Also, the new validator is only used to verify
> loop bounds. Memory accesses are still handled by the old validator.
>
> Modified:
>    polly/trunk/lib/Analysis/ScopDetection.cpp
>
> Modified: polly/trunk/lib/Analysis/ScopDetection.cpp
> URL: http://llvm.org/viewvc/llvm-project/polly/trunk/lib/Analysis/ScopDetection.cpp?rev=143576&r1=143575&r2=143576&view=diff
> ==============================================================================
> --- polly/trunk/lib/Analysis/ScopDetection.cpp (original)
> +++ polly/trunk/lib/Analysis/ScopDetection.cpp Wed Nov  2 16:40:08 2011
> @@ -109,6 +109,162 @@
>  //===----------------------------------------------------------------------===//
>  // ScopDetection.
>
> +namespace SCEVType {
> +  enum TYPE {INT, PARAM, IV, INVALID};
> +}
> +
> +/// Check if a SCEV is valid in a SCoP.
> +struct SCEVValidator : public SCEVVisitor<SCEVValidator, SCEVType::TYPE> {
> +private:
> +  const Region *R;
> +  ScalarEvolution &SE;
> +  const Value **BaseAddress;
> +
> +public:
> +  static bool isValid(const Region *R, const SCEV *Scev,
> +                      ScalarEvolution &SE,
> +                      const Value **BaseAddress = NULL) {
> +    if (isa<SCEVCouldNotCompute>(Scev))
> +      return false;
> +
> +    SCEVValidator Validator(R, SE, BaseAddress);
> +    return Validator.visit(Scev) != SCEVType::INVALID;
> +  }
> +
> +  SCEVValidator(const Region *R, ScalarEvolution &SE,
> +                const Value **BaseAddress) : R(R), SE(SE),
> +    BaseAddress(BaseAddress) {};
> +
> +  SCEVType::TYPE visitConstant(const SCEVConstant *Constant) {
> +    return SCEVType::INT;
> +  }
> +
> +  SCEVType::TYPE visitTruncateExpr(const SCEVTruncateExpr* Expr) {

Please use a single way to place the *, i.e., SCEVTruncateExpr *Expr
also occurs below, several times in several inconsistent ways.

> +    SCEVType::TYPE Op = visit(Expr->getOperand());
> +
> +    // We cannot represent this as a affine expression yet. If it is constant

s/a affine/an affine/

> +    // during Scop execution treat this as a parameter, otherwise bail out.
> +    if (Op == SCEVType::INT || Op == SCEVType::PARAM)
> +      return SCEVType::PARAM;
> +
> +    return SCEVType::INVALID;
> +  }
> +
> +  SCEVType::TYPE visitZeroExtendExpr(const SCEVZeroExtendExpr * Expr) {
> +    SCEVType::TYPE Op = visit(Expr->getOperand());
> +
> +    // We cannot represent this as a affine expression yet. If it is constant

s/a affine/an affine/

> +    // during Scop execution treat this as a parameter, otherwise bail out.
> +    if (Op == SCEVType::INT || Op == SCEVType::PARAM)
> +      return SCEVType::PARAM;
> +
> +    return SCEVType::INVALID;
> +  }
> +
> +  SCEVType::TYPE visitSignExtendExpr(const SCEVSignExtendExpr* Expr) {
> +    // Assuming the value is signed, a sign extension is basically a noop.
> +    // TODO: Reconsider this as soon as we support unsigned values.
> +    return visit(Expr->getOperand());
> +  }
> +
> +  SCEVType::TYPE visitAddExpr(const SCEVAddExpr* Expr) {
> +    SCEVType::TYPE Return = SCEVType::INT;
> +
> +    for (int i = 0, e = Expr->getNumOperands(); i < e; ++i) {
> +      SCEVType::TYPE OpType = visit(Expr->getOperand(i));
> +      Return = std::max(Return, OpType);
> +    }
> +
> +    // TODO: Check for NSW and NUW.
> +    return Return;
> +  }
> +
> +  SCEVType::TYPE visitMulExpr(const SCEVMulExpr* Expr) {
> +    SCEVType::TYPE Return = SCEVType::INT;
> +
> +    for (int i = 0, e = Expr->getNumOperands(); i < e; ++i) {
> +      SCEVType::TYPE OpType = visit(Expr->getOperand(i));
> +
> +      if (OpType == SCEVType::INVALID)
> +        return SCEVType::INVALID;
> +
> +      if (OpType == SCEVType::IV) {
> +        if (Return == SCEVType::PARAM || Return == SCEVType::IV)
> +          return SCEVType::INVALID;
> +
> +        Return = OpType;
> +        continue;
> +      }
> +
> +      if (OpType == SCEVType::PARAM) {
> +        if (Return == SCEVType::PARAM)
> +          return SCEVType::INVALID;
> +
> +        Return = SCEVType::PARAM;
> +        continue;
> +      }
> +
> +      // OpType == SCEVType::INT, no need to change anything.
> +    }
> +
> +    // TODO: Check for NSW and NUW.
> +    return Return;
> +  }
> +
> +  SCEVType::TYPE visitUDivExpr(const SCEVUDivExpr* Expr) {
> +    // We do not yet support unsigned operations.
> +    return SCEVType::INVALID;
> +  }
> +
> +  SCEVType::TYPE visitAddRecExpr(const SCEVAddRecExpr* Expr) {
> +    if (!Expr->isAffine())
> +      return SCEVType::INVALID;
> +
> +    SCEVType::TYPE Start = visit(Expr->getStart());
> +
> +    if (Start == SCEVType::INVALID)
> +      return Start;
> +
> +    SCEVType::TYPE Recurrence = visit(Expr->getStepRecurrence(SE));

In the following code, why not doing this instead?


   if (Recurrence == SCEVType::INT)
     return SCEVType::IV;

   return SCEVType::INVALID;

> +    if (Recurrence != SCEVType::INT)
> +      return SCEVType::INVALID;
> +
> +    return SCEVType::PARAM;

I don't see why you would return a PARAM for an addrec with a constant step.
I would return SCEVType::IV.

> +  }
> +
> +  SCEVType::TYPE visitSMaxExpr(const SCEVSMaxExpr* Expr) {
> +    SCEVType::TYPE Return = SCEVType::INT;
> +
> +    for (int i = 0, e = Expr->getNumOperands(); i < e; ++i) {
> +      SCEVType::TYPE OpType = visit(Expr->getOperand(i));
> +
> +      if (OpType == SCEVType::INVALID)
> +        return SCEVType::INVALID;
> +      if (OpType == SCEVType::PARAM)
> +        Return = SCEVType::PARAM;
> +    }
> +
> +    return Return;
> +  }
> +
> +  SCEVType::TYPE visitUMaxExpr(const SCEVUMaxExpr* Expr) {
> +    // We do not yet support unsigned operations. If 'Expr' is constant
> +    // during Scop execution treat this as a parameter, otherwise bail out.
> +    for (int i = 0, e = Expr->getNumOperands(); i < e; ++i) {
> +      SCEVType::TYPE OpType = visit(Expr->getOperand(i));
> +
> +      if (OpType != SCEVType::INT && OpType != SCEVType::PARAM)
> +        return SCEVType::PARAM;

Probably this first return should have been SCEVType::INVALID?

> +    }
> +
> +    return SCEVType::PARAM;
> +  }

You should check the logic in this function, as I see that you always
return SCEVType::PARAM.


> +
> +  SCEVType::TYPE visitUnknown(const SCEVUnknown* Expr) {
> +    return SCEVType::PARAM;
> +  }

Really?  I would have returned INVALID instead of PARAM for an unknown SCEV.

> +};
> +
>  bool ScopDetection::isMaxRegionInScop(const Region &R) const {
>   // The Region is valid only if it could be found in the set.
>   return ValidRegions.count(&R);
> @@ -401,7 +557,7 @@
>
>   // Is the loop count affine?
>   const SCEV *LoopCount = SE->getBackedgeTakenCount(L);
> -  if (!isValidAffineFunction(LoopCount, Context.CurRegion))
> +  if (!SCEVValidator::isValid(&Context.CurRegion, LoopCount, *SE))
>     INVALID(LoopBound, "Non affine loop bound '" << *LoopCount << "' in loop: "
>                        << L->getHeader()->getNameStr());
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>



-- 
Sebastian Pop
--
Qualcomm Innovation Center, Inc is a member of Code Aurora Forum