[polly][PATCH] Support the new DiagnosticRemarks (v2)

David Peixotto dpeixott at codeaurora.org
Fri Jun 13 15:45:19 PDT 2014


Hi Andreas,

Overall the direction of this patch looks good to me. It would be easier 
to review a large patch like this using phabricator, so please consider 
using that tool next time.

Main comments:
   * Use ovveride when overriding virtual methods (e.g. for RejectReason 
methods)
   * Verify that getDebugLoc() in RejectReason is correct. Looks like it 
might be returning an invalid reference to me. Can either return by 
value or add a default DebugLoc variable to RejectReason class.

A few other comments/questions below.

On 6/12/2014 12:44 AM, Andreas Simbuerger wrote:
> Add support for generating optimization remarks after completing the
> detection of Scops.
> The goal is to provide end-users with useful hints about opportunities that
> help to increase the size of the detected Scops in their code.
>
> By default the remark is unspecified and the debug location is empty. Future
> patches have to expand on the messages generated.
>
> This patch brings a simple test case for ReportFuncCall to demonstrate the
> feature.
>
> Usage:
>   clang <...> -Rpass="polly-detect" <...>
>   opt <...> -pass-remarks="polly-detect" <...>
> ---
>   include/polly/ScopDetection.h                      |  17 ++
>   include/polly/ScopDetectionDiagnostic.h            | 138 +++++++++----
>   lib/Analysis/ScopDetection.cpp                     |  74 +++++--
>   lib/Analysis/ScopDetectionDiagnostic.cpp           | 214 ++++++++++++++++++++-
>   test/ScopDetectionDiagnostics/ReportFuncCall-01.ll |  67 +++++++
>   5 files changed, 448 insertions(+), 62 deletions(-)
>   create mode 100644 test/ScopDetectionDiagnostics/ReportFuncCall-01.ll
>
> diff --git a/include/polly/ScopDetection.h b/include/polly/ScopDetection.h
> index 5bab520..7db51ee 100644
> --- a/include/polly/ScopDetection.h
> +++ b/include/polly/ScopDetection.h
> @@ -326,6 +326,23 @@ public:
>     const_reject_iterator reject_end() const { return RejectLogs.end(); }
>     //@}
>
> +  /// @brief Emit rejection remarks for all smallest invalid regions.
> +  ///
> +  /// @param F The function to emit remarks for.
> +  /// @param R The region to start the region tree traversal for.
> +  void emitRemarksForLeaves(const Function &F, const Region *R);
> +
> +  /// @brief Emit rejection remarks for the parent regions of all valid regions.
> +  ///
> +  /// Emitting rejection remarks for the parent regions of all valid regions
> +  /// may give the end-user clues about how to increase the size of the
> +  /// detected Scops.
> +  ///
> +  /// @param F The function to emit remarks for.
> +  /// @param ValidRegions The set of valid regions to emit remarks for.
> +  void emitRemarksForValidRegions(const Function &F,
> +                                  const RegionSet &ValidRegions);
> +
>     /// @brief Mark the function as invalid so we will not extract any scop from
>     ///        the function.
>     ///
> diff --git a/include/polly/ScopDetectionDiagnostic.h b/include/polly/ScopDetectionDiagnostic.h
> index 1006770..b0c364a 100644
> --- a/include/polly/ScopDetectionDiagnostic.h
> +++ b/include/polly/ScopDetectionDiagnostic.h
> @@ -52,6 +52,16 @@ namespace polly {
>   void getDebugLocation(const Region *R, unsigned &LineBegin, unsigned &LineEnd,
>                         std::string &FileName);
>
> +class RejectLog;
> +/// @brief Emit optimization remarks about the rejected regions to the user.
> +///
> +/// This emits the content of the reject log as optimization remarks.
> +/// Remember to at least track failures (-polly-detect-track-failures).
> +/// @param F The function we emit remarks for.
> +/// @param Log The error log containing all messages being emitted as remark.
> +/// @param LI LoopInfo to fetch the surrounding loops.
> +void emitRejectionRemarks(const llvm::Function &F, const RejectLog &Log);
> +
>   //===----------------------------------------------------------------------===//
>   /// @brief Base class of all reject reasons found during Scop detection.
>   ///
> @@ -67,6 +77,23 @@ public:
>     ///
>     /// @return A debug message representing this error.
>     virtual std::string getMessage() const = 0;
> +
> +  /// @brief Generate a message for the end-user describing this error.
> +  ///
> +  /// The message provided has to be suitable for the end-user. So it should
> +  /// not reference any LLVM internal data structures or terminology.
> +  /// Ideally, the message helps the end-user to increase the size of the
> +  /// regions amenable to Polly.
> +  ///
> +  /// @return A short message representing this error.
> +  virtual std::string getEndUserMessage() const {
> +    return "Unspecified error.";
> +  };
> +
> +  /// @brief Get the source location of this error.
> +  ///
> +  /// @return The debug location for this error.
> +  virtual const llvm::DebugLoc &getDebugLoc() const;
>   };
>
>   typedef std::shared_ptr<RejectReason> RejectReasonPtr;
> @@ -79,10 +106,10 @@ class RejectLog {
>   public:
>     explicit RejectLog(Region *R) : R(R) {}
>
> -  typedef llvm::SmallVector<RejectReasonPtr, 1>::iterator iterator;
> +  typedef llvm::SmallVector<RejectReasonPtr, 1>::const_iterator iterator;
>
> -  iterator begin() { return ErrorReports.begin(); }
> -  iterator end() { return ErrorReports.end(); }
> +  iterator begin() const { return ErrorReports.begin(); }
> +  iterator end() const { return ErrorReports.end(); }
>     size_t size() { return ErrorReports.size(); }
>
>     const Region *region() const { return R; }
> @@ -104,11 +131,12 @@ class ReportNonBranchTerminator : public ReportCFG {
>     BasicBlock *BB;
>
>   public:
> -  ReportNonBranchTerminator(BasicBlock *BB) : BB(BB) {}
> +  ReportNonBranchTerminator(BasicBlock *BB) : ReportCFG(), BB(BB) {}
>
>     /// @name RejectReason interface
>     //@{
>     virtual std::string getMessage() const;
> +  virtual const DebugLoc &getDebugLoc() const;
>     //@}
>   };
>
> @@ -121,11 +149,12 @@ class ReportCondition : public ReportCFG {
>     BasicBlock *BB;
>
>   public:
> -  ReportCondition(BasicBlock *BB) : BB(BB) {}
> +  ReportCondition(BasicBlock *BB) : ReportCFG(), BB(BB) {}
>
>     /// @name RejectReason interface
>     //@{
>     virtual std::string getMessage() const;
> +  virtual const DebugLoc &getDebugLoc() const;
>     //@}
>   };
>
> @@ -136,8 +165,14 @@ public:
>   /// this class.
>   class ReportAffFunc : public RejectReason {
>     //===--------------------------------------------------------------------===//
> +
> +  // The instruction that caused non-affinity to occur.
> +  const Instruction *Inst;
> +
>   public:
> -  ReportAffFunc();
> +  ReportAffFunc(const Instruction *Inst);
> +
> +  virtual const DebugLoc &getDebugLoc() const { return Inst->getDebugLoc(); };
>   };
>
>   //===----------------------------------------------------------------------===//
> @@ -149,7 +184,8 @@ class ReportUndefCond : public ReportAffFunc {
>     BasicBlock *BB;
>
>   public:
> -  ReportUndefCond(BasicBlock *BB) : BB(BB) {}
> +  ReportUndefCond(const Instruction *Inst, BasicBlock *BB)
> +      : ReportAffFunc(Inst), BB(BB) {}
>
>     /// @name RejectReason interface
>     //@{
> @@ -168,7 +204,8 @@ class ReportInvalidCond : public ReportAffFunc {
>     BasicBlock *BB;
>
>   public:
> -  ReportInvalidCond(BasicBlock *BB) : BB(BB) {}
> +  ReportInvalidCond(const Instruction *Inst, BasicBlock *BB)
> +      : ReportAffFunc(Inst), BB(BB) {}
>
>     /// @name RejectReason interface
>     //@{
> @@ -185,7 +222,8 @@ class ReportUndefOperand : public ReportAffFunc {
>     BasicBlock *BB;
>
>   public:
> -  ReportUndefOperand(BasicBlock *BB) : BB(BB) {}
> +  ReportUndefOperand(BasicBlock *BB, const Instruction *Inst)
> +      : ReportAffFunc(Inst), BB(BB) {}
>
>     /// @name RejectReason interface
>     //@{
> @@ -208,8 +246,9 @@ class ReportNonAffBranch : public ReportAffFunc {
>     //@}
>
>   public:
> -  ReportNonAffBranch(BasicBlock *BB, const SCEV *LHS, const SCEV *RHS)
> -      : BB(BB), LHS(LHS), RHS(RHS) {}
> +  ReportNonAffBranch(BasicBlock *BB, const SCEV *LHS, const SCEV *RHS,
> +                     const Instruction *Inst)
> +      : ReportAffFunc(Inst), BB(BB), LHS(LHS), RHS(RHS) {}
>
>     const SCEV *lhs() { return LHS; }
>     const SCEV *rhs() { return RHS; }
> @@ -225,6 +264,8 @@ public:
>   class ReportNoBasePtr : public ReportAffFunc {
>     //===--------------------------------------------------------------------===//
>   public:
> +  ReportNoBasePtr(const Instruction *Inst) : ReportAffFunc(Inst) {}
> +
>     /// @name RejectReason interface
>     //@{
>     virtual std::string getMessage() const;
> @@ -236,6 +277,8 @@ public:
>   class ReportUndefBasePtr : public ReportAffFunc {
>     //===--------------------------------------------------------------------===//
>   public:
> +  ReportUndefBasePtr(const Instruction *Inst) : ReportAffFunc(Inst) {}
> +
>     /// @name RejectReason interface
>     //@{
>     virtual std::string getMessage() const;
> @@ -251,7 +294,8 @@ class ReportVariantBasePtr : public ReportAffFunc {
>     Value *BaseValue;
>
>   public:
> -  ReportVariantBasePtr(Value *BaseValue) : BaseValue(BaseValue) {}
> +  ReportVariantBasePtr(Value *BaseValue, const Instruction *Inst)
> +      : ReportAffFunc(Inst), BaseValue(BaseValue) {}
>
>     /// @name RejectReason interface
>     //@{
> @@ -268,8 +312,8 @@ class ReportNonAffineAccess : public ReportAffFunc {
>     const SCEV *AccessFunction;
>
>   public:
> -  ReportNonAffineAccess(const SCEV *AccessFunction)
> -      : AccessFunction(AccessFunction) {}
> +  ReportNonAffineAccess(const SCEV *AccessFunction, const Instruction *Inst)
> +      : ReportAffFunc(Inst), AccessFunction(AccessFunction) {}
>
>     const SCEV *get() { return AccessFunction; }
>
> @@ -299,11 +343,12 @@ class ReportPhiNodeRefInRegion : public ReportIndVar {
>     Instruction *Inst;
>
>   public:
> -  ReportPhiNodeRefInRegion(Instruction *Inst) : Inst(Inst) {}
> +  ReportPhiNodeRefInRegion(Instruction *Inst);
>
>     /// @name RejectReason interface
>     //@{
>     virtual std::string getMessage() const;
> +  virtual const DebugLoc &getDebugLoc() const;
>     //@}
>   };
>
> @@ -316,11 +361,12 @@ class ReportNonCanonicalPhiNode : public ReportIndVar {
>     Instruction *Inst;
>
>   public:
> -  ReportNonCanonicalPhiNode(Instruction *Inst) : Inst(Inst) {}
> +  ReportNonCanonicalPhiNode(Instruction *Inst);
>
>     /// @name RejectReason interface
>     //@{
>     virtual std::string getMessage() const;
> +  virtual const DebugLoc &getDebugLoc() const;
>     //@}
>   };
>
> @@ -333,11 +379,12 @@ class ReportLoopHeader : public ReportIndVar {
>     Loop *L;
>
>   public:
> -  ReportLoopHeader(Loop *L) : L(L) {}
> +  ReportLoopHeader(Loop *L);
>
>     /// @name RejectReason interface
>     //@{
>     virtual std::string getMessage() const;
> +  virtual const DebugLoc &getDebugLoc() const;
>     //@}
>   };
>
> @@ -345,12 +392,16 @@ public:
>   /// @brief Captures a region with invalid entering edges.
>   class ReportIndEdge : public RejectReason {
>     //===--------------------------------------------------------------------===//
> +
> +  BasicBlock *BB;
> +
>   public:
> -  ReportIndEdge();
> +  ReportIndEdge(BasicBlock *BB);
>
>     /// @name RejectReason interface
>     //@{
>     virtual std::string getMessage() const;
> +  virtual const DebugLoc &getDebugLoc() const;
>     //@}
>   };
>
> @@ -373,6 +424,7 @@ public:
>     /// @name RejectReason interface
>     //@{
>     virtual std::string getMessage() const;
> +  virtual const DebugLoc &getDebugLoc() const;
>     //@}
>   };
>
> @@ -389,7 +441,9 @@ public:
>
>     /// @name RejectReason interface
>     //@{
> -  std::string getMessage() const;
> +  virtual std::string getMessage() const;
> +  virtual const DebugLoc &getDebugLoc() const;
> +  virtual std::string getEndUserMessage() const;
>     //@}
>   };
>
> @@ -398,20 +452,21 @@ public:
>   class ReportAlias : public RejectReason {
>     //===--------------------------------------------------------------------===//
>
> -  // The offending alias set.
> -  AliasSet *AS;
> -
>     /// @brief Format an invalid alias set.
>     ///
>     /// @param AS The invalid alias set to format.
>     std::string formatInvalidAlias(AliasSet &AS) const;
>
> +  AliasSet *AS;
> +  Instruction *Inst;
> +
>   public:
> -  ReportAlias(AliasSet *AS);
> +  ReportAlias(Instruction *Inst, AliasSet *AS);
>
>     /// @name RejectReason interface
>     //@{
> -  std::string getMessage() const;
> +  virtual std::string getMessage() const;
> +  virtual const DebugLoc &getDebugLoc() const;
>     //@}
>   };
>
> @@ -424,7 +479,7 @@ public:
>
>     /// @name RejectReason interface
>     //@{
> -  std::string getMessage() const;
> +  virtual std::string getMessage() const;
>     //@}
>   };
>
> @@ -437,7 +492,7 @@ public:
>
>     /// @name RejectReason interface
>     //@{
> -  std::string getMessage() const { return "Unknown reject reason"; }
> +  virtual std::string getMessage() const;
>     //@}
>   };
>
> @@ -447,14 +502,15 @@ class ReportIntToPtr : public ReportOther {
>     //===--------------------------------------------------------------------===//
>
>     // The offending base value.
> -  Value *BaseValue;
> +  Instruction *BaseValue;
>
>   public:
> -  ReportIntToPtr(Value *BaseValue) : BaseValue(BaseValue) {}
> +  ReportIntToPtr(Instruction *BaseValue);
>
>     /// @name RejectReason interface
>     //@{
> -  std::string getMessage() const;
> +  virtual std::string getMessage() const;
> +  virtual const DebugLoc &getDebugLoc() const;
>     //@}
>   };
>
> @@ -465,11 +521,12 @@ class ReportAlloca : public ReportOther {
>     Instruction *Inst;
>
>   public:
> -  ReportAlloca(Instruction *Inst) : Inst(Inst) {}
> +  ReportAlloca(Instruction *Inst);
>
>     /// @name RejectReason interface
>     //@{
> -  std::string getMessage() const;
> +  virtual std::string getMessage() const;
> +  virtual const DebugLoc &getDebugLoc() const;
>     //@}
>   };
>
> @@ -480,11 +537,12 @@ class ReportUnknownInst : public ReportOther {
>     Instruction *Inst;
>
>   public:
> -  ReportUnknownInst(Instruction *Inst) : Inst(Inst) {}
> +  ReportUnknownInst(Instruction *Inst);
>
>     /// @name RejectReason interface
>     //@{
> -  std::string getMessage() const;
> +  virtual std::string getMessage() const;
> +  virtual const DebugLoc &getDebugLoc() const;
>     //@}
>   };
>
> @@ -492,10 +550,15 @@ public:
>   /// @brief Captures errors with phi nodes in exit BBs.
>   class ReportPHIinExit : public ReportOther {
>     //===--------------------------------------------------------------------===//
> +  Instruction *Inst;
> +
>   public:
> +  ReportPHIinExit(Instruction *Inst);
> +
>     /// @name RejectReason interface
>     //@{
> -  std::string getMessage() const;
> +  virtual std::string getMessage() const;
> +  virtual const DebugLoc &getDebugLoc() const;
>     //@}
>   };
>
> @@ -503,10 +566,15 @@ public:
>   /// @brief Captures errors with regions containing the function entry block.
>   class ReportEntry : public ReportOther {
>     //===--------------------------------------------------------------------===//
> +  BasicBlock *BB;
> +
>   public:
> +  ReportEntry(BasicBlock *BB);
> +
>     /// @name RejectReason interface
>     //@{
> -  std::string getMessage() const;
> +  virtual std::string getMessage() const;
> +  virtual const DebugLoc &getDebugLoc() const;
>     //@}
>   };
>
> diff --git a/lib/Analysis/ScopDetection.cpp b/lib/Analysis/ScopDetection.cpp
> index f1fafaa..01217bb 100644
> --- a/lib/Analysis/ScopDetection.cpp
> +++ b/lib/Analysis/ScopDetection.cpp
> @@ -250,11 +250,11 @@ bool ScopDetection::isValidCFG(BasicBlock &BB,
>
>     // UndefValue is not allowed as condition.
>     if (isa<UndefValue>(Condition))
> -    return invalid<ReportUndefCond>(Context, /*Assert=*/true, &BB);
> +    return invalid<ReportUndefCond>(Context, /*Assert=*/true, Br, &BB);
>
>     // Only Constant and ICmpInst are allowed as condition.
>     if (!(isa<Constant>(Condition) || isa<ICmpInst>(Condition)))
> -    return invalid<ReportInvalidCond>(Context, /*Assert=*/true, &BB);
> +    return invalid<ReportInvalidCond>(Context, /*Assert=*/true, Br, &BB);
>
>     // Allow perfectly nested conditions.
>     assert(Br->getNumSuccessors() == 2 && "Unexpected number of successors");
> @@ -271,7 +271,7 @@ bool ScopDetection::isValidCFG(BasicBlock &BB,
>       // Are both operands of the ICmp affine?
>       if (isa<UndefValue>(ICmp->getOperand(0)) ||
>           isa<UndefValue>(ICmp->getOperand(1)))
> -      return invalid<ReportUndefOperand>(Context, /*Assert=*/true, &BB);
> +      return invalid<ReportUndefOperand>(Context, /*Assert=*/true, &BB, ICmp);
>
>       Loop *L = LI->getLoopFor(ICmp->getParent());
>       const SCEV *LHS = SE->getSCEVAtScope(ICmp->getOperand(0), L);
> @@ -280,7 +280,7 @@ bool ScopDetection::isValidCFG(BasicBlock &BB,
>       if (!isAffineExpr(&Context.CurRegion, LHS, *SE) ||
>           !isAffineExpr(&Context.CurRegion, RHS, *SE))
>         return invalid<ReportNonAffBranch>(Context, /*Assert=*/true, &BB, LHS,
> -                                         RHS);
> +                                         RHS, ICmp);
>     }
>
>     // Allow loop exit conditions.
> @@ -380,8 +380,8 @@ bool ScopDetection::hasAffineMemoryAccesses(DetectionContext &Context) const {
>         const SCEVAddRecExpr *AF = PIAF.second;
>         const Instruction *Insn = PIAF.first;
>         if (Shape->DelinearizedSizes.empty())
> -        return invalid<ReportNonAffineAccess>(Context, /*Assert=*/true,
> -                                              PIAF.second);
> +        return invalid<ReportNonAffineAccess>(Context, /*Assert=*/true, AF,
> +                                              Insn);
>
>         MemAcc *Acc = new MemAcc(Insn, Shape);
>         InsnToMemAcc.insert({Insn, Acc});
> @@ -389,12 +389,14 @@ bool ScopDetection::hasAffineMemoryAccesses(DetectionContext &Context) const {
>                                    Shape->DelinearizedSizes);
>         if (Shape->DelinearizedSizes.empty() ||
>             Acc->DelinearizedSubscripts.empty())
> -        return invalid<ReportNonAffineAccess>(Context, /*Assert=*/true, AF);
> +        return invalid<ReportNonAffineAccess>(Context, /*Assert=*/true, AF,
> +                                              Insn);
>
>         // Check that the delinearized subscripts are affine.
>         for (const SCEV *S : Acc->DelinearizedSubscripts)
>           if (!isAffineExpr(&Context.CurRegion, S, *SE, BaseValue))
> -          return invalid<ReportNonAffineAccess>(Context, /*Assert=*/true, AF);
> +          return invalid<ReportNonAffineAccess>(Context, /*Assert=*/true, AF,
> +                                                Insn);
>       }
>     }
>     return true;
> @@ -411,12 +413,12 @@ bool ScopDetection::isValidMemoryAccess(Instruction &Inst,
>     BasePointer = dyn_cast<SCEVUnknown>(SE->getPointerBase(AccessFunction));
>
>     if (!BasePointer)
> -    return invalid<ReportNoBasePtr>(Context, /*Assert=*/true);
> +    return invalid<ReportNoBasePtr>(Context, /*Assert=*/true, &Inst);
>
>     BaseValue = BasePointer->getValue();
>
>     if (isa<UndefValue>(BaseValue))
> -    return invalid<ReportUndefBasePtr>(Context, /*Assert=*/true);
> +    return invalid<ReportUndefBasePtr>(Context, /*Assert=*/true, &Inst);
>
>     // Check that the base address of the access is invariant in the current
>     // region.
> @@ -424,7 +426,8 @@ bool ScopDetection::isValidMemoryAccess(Instruction &Inst,
>       // Verification of this property is difficult as the independent blocks
>       // pass may introduce aliasing that we did not have when running the
>       // scop detection.
> -    return invalid<ReportVariantBasePtr>(Context, /*Assert=*/false, BaseValue);
> +    return invalid<ReportVariantBasePtr>(Context, /*Assert=*/false, BaseValue,
> +                                         &Inst);
>
>     AccessFunction = SE->getMinusSCEV(AccessFunction, BasePointer);
>
> @@ -436,7 +439,7 @@ bool ScopDetection::isValidMemoryAccess(Instruction &Inst,
>
>       if (!PollyDelinearize || !AF)
>         return invalid<ReportNonAffineAccess>(Context, /*Assert=*/true,
> -                                            AccessFunction);
> +                                            AccessFunction, &Inst);
>
>       const SCEV *ElementSize = SE->getElementSize(&Inst);
>       Context.ElementSize[BasePointer] = ElementSize;
> @@ -456,8 +459,8 @@ bool ScopDetection::isValidMemoryAccess(Instruction &Inst,
>
>     // FIXME: Alias Analysis thinks IntToPtrInst aliases with alloca instructions
>     // created by IndependentBlocks Pass.
> -  if (isa<IntToPtrInst>(BaseValue))
> -    return invalid<ReportIntToPtr>(Context, /*Assert=*/true, BaseValue);
> +  if (IntToPtrInst *Inst = dyn_cast<IntToPtrInst>(BaseValue))
> +    return invalid<ReportIntToPtr>(Context, /*Assert=*/true, Inst);
>
>     if (IgnoreAliasing)
>       return true;
> @@ -476,7 +479,7 @@ bool ScopDetection::isValidMemoryAccess(Instruction &Inst,
>     // not proof this without -basicaa we would fail. We disable this check to
>     // not cause irrelevant verification failures.
>     if (!AS.isMustAlias())
> -    return invalid<ReportAlias>(Context, /*Assert=*/true, &AS);
> +    return invalid<ReportAlias>(Context, /*Assert=*/true, &Inst, &AS);
>
>     return true;
>   }
> @@ -692,7 +695,7 @@ bool ScopDetection::isValidExit(DetectionContext &Context) const {
>     if (BasicBlock *Exit = R.getExit()) {
>       BasicBlock::iterator I = Exit->begin();
>       if (I != Exit->end() && isa<PHINode>(*I))
> -      return invalid<ReportPHIinExit>(Context, /*Assert=*/true);
> +      return invalid<ReportPHIinExit>(Context, /*Assert=*/true, I);
>     }
>
>     return true;
> @@ -745,7 +748,7 @@ bool ScopDetection::isValidRegion(DetectionContext &Context) const {
>           // Region entering edges come from the same loop but outside the region
>           // are not allowed.
>           if (L->contains(*PI) && !R.contains(*PI))
> -          return invalid<ReportIndEdge>(Context, /*Assert=*/true);
> +          return invalid<ReportIndEdge>(Context, /*Assert=*/true, *PI);
>         }
>       }
>     }
> @@ -753,7 +756,7 @@ bool ScopDetection::isValidRegion(DetectionContext &Context) const {
>     // SCoP cannot contain the entry block of the function, because we need
>     // to insert alloca instruction there when translate scalar to array.
>     if (R.getEntry() == &(R.getEntry()->getParent()->getEntryBlock()))
> -    return invalid<ReportEntry>(Context, /*Assert=*/true);
> +    return invalid<ReportEntry>(Context, /*Assert=*/true, R.getEntry());
>
>     if (!isValidExit(Context))
>       return false;
> @@ -780,6 +783,35 @@ void ScopDetection::printLocations(llvm::Function &F) {
>     }
>   }
>
> +void ScopDetection::emitRemarksForValidRegions(const Function &F,
> +                                               const RegionSet &ValidRegions) {
What is the type of RegionSet? Will it produce deterministic output 
(i.e. always iterate over the regions in the same order)?

> +  for (const Region *R : ValidRegions) {
> +    const Region *Parent = R->getParent();
> +    if (Parent && !Parent->isTopLevelRegion() && RejectLogs.count(Parent))
> +      emitRejectionRemarks(F, RejectLogs.at(Parent));
> +  }
> +}
> +
> +void ScopDetection::emitRemarksForLeaves(const Function &F, const Region *R) {
> +  for (Region::const_iterator ChildIt = R->begin(), End = R->end();
> +       ChildIt != End; ++ChildIt) {
> +    const std::unique_ptr<Region> &Child = *ChildIt;
> +
> +    bool IsValid = ValidRegions.count(Child.get());
> +    if (IsValid)
> +      continue;
> +
> +    bool IsLeaf = Child->begin() == Child->end();
> +    if (!IsLeaf)
> +      emitRemarksForLeaves(F, Child.get());
> +    else {
> +      if (RejectLogs.count(Child.get())) {
> +        emitRejectionRemarks(F, RejectLogs.at(Child.get()));
> +      }
> +    }
> +  }
> +}
> +
>   bool ScopDetection::runOnFunction(llvm::Function &F) {
>     LI = &getAnalysis<LoopInfo>();
>     RI = &getAnalysis<RegionInfo>();
> @@ -800,6 +832,12 @@ bool ScopDetection::runOnFunction(llvm::Function &F) {
>
>     findScops(*TopRegion);
>
> +  // Only makes sense when we tracked errors.
> +  if (PollyTrackFailures) {
> +    emitRemarksForValidRegions(F, ValidRegions);
> +    emitRemarksForLeaves(F, TopRegion);
> +  }
> +
>     if (ReportLevel >= 1)
>       printLocations(F);
>
> diff --git a/lib/Analysis/ScopDetectionDiagnostic.cpp b/lib/Analysis/ScopDetectionDiagnostic.cpp
> index 0fa7ba7..1bdcd74 100644
> --- a/lib/Analysis/ScopDetectionDiagnostic.cpp
> +++ b/lib/Analysis/ScopDetectionDiagnostic.cpp
> @@ -22,12 +22,15 @@
>   #include "llvm/Analysis/LoopInfo.h"
>   #include "llvm/Analysis/AliasSetTracker.h"
>   #include "llvm/IR/BasicBlock.h"
> +#include "llvm/IR/DebugInfo.h"
> +#include "llvm/IR/DebugLoc.h"
> +#include "llvm/IR/DiagnosticInfo.h"
> +#include "llvm/IR/LLVMContext.h"
>   #include "llvm/IR/Value.h"
>   #include "llvm/ADT/SmallVector.h"
>   #include "llvm/ADT/Statistic.h"
>
>   #include "llvm/Analysis/RegionInfo.h"
> -#include "llvm/IR/DebugInfo.h"
>
>   #define DEBUG_TYPE "polly-detect"
>   #include "llvm/Support/Debug.h"
> @@ -81,75 +84,187 @@ void getDebugLocation(const Region *R, unsigned &LineBegin, unsigned &LineEnd,
>       }
>   }
>
> +void emitRejectionRemarks(const llvm::Function &F, const RejectLog &Log) {
> +  LLVMContext &Ctx = F.getContext();
> +
> +  const Region *R = Log.region();
> +  const BasicBlock *Entry = R->getEntry();
> +  DebugLoc DL = Entry->getTerminator()->getDebugLoc();
> +  Ctx.diagnose(DiagnosticInfoOptimizationRemark(
> +      DEBUG_TYPE, F, DL,
> +      "The following errors keep this region from being a Scop."));
> +  for (RejectReasonPtr RR : Log) {
> +    Ctx.diagnose(DiagnosticInfoOptimizationRemark(
> +        DEBUG_TYPE, F, RR->getDebugLoc(), RR->getEndUserMessage()));
> +  }
> +}
> +
> +//===----------------------------------------------------------------------===//
> +// RejectReason.
> +
> +const llvm::DebugLoc &RejectReason::getDebugLoc() const {
> +  // Allocate an empty DebugLoc and return it a reference to it.
> +  return *(std::make_shared<DebugLoc>().get());
> +}
> +
Why is this using a shared_ptr, but returning a reference? Won't this 
return a dangling reference?

>   //===----------------------------------------------------------------------===//
>   // ReportCFG.
>
>   ReportCFG::ReportCFG() { ++BadCFGForScop; }
>
> +//===----------------------------------------------------------------------===//
> +// ReportNonBranchTerminator.
> +
>   std::string ReportNonBranchTerminator::getMessage() const {
>     return ("Non branch instruction terminates BB: " + BB->getName()).str();
>   }
>
> +const DebugLoc &ReportNonBranchTerminator::getDebugLoc() const {
> +  return BB->getTerminator()->getDebugLoc();
> +}
> +
> +//===----------------------------------------------------------------------===//
> +// ReportCondition.
> +
>   std::string ReportCondition::getMessage() const {
>     return ("Not well structured condition at BB: " + BB->getName()).str();
>   }
>
> -ReportAffFunc::ReportAffFunc() { ++BadAffFuncForScop; }
> +const DebugLoc &ReportCondition::getDebugLoc() const {
> +  return BB->getTerminator()->getDebugLoc();
> +}
> +
> +//===----------------------------------------------------------------------===//
> +// ReportAffFunc.
> +
> +ReportAffFunc::ReportAffFunc(const Instruction *Inst)
> +    : RejectReason(), Inst(Inst) {
> +  ++BadAffFuncForScop;
> +}
> +
> +//===----------------------------------------------------------------------===//
> +// ReportUndefCond.
>
>   std::string ReportUndefCond::getMessage() const {
>     return ("Condition based on 'undef' value in BB: " + BB->getName()).str();
>   }
>
> +//===----------------------------------------------------------------------===//
> +// ReportInvalidCond.
> +
>   std::string ReportInvalidCond::getMessage() const {
>     return ("Condition in BB '" + BB->getName()).str() +
>            "' neither constant nor an icmp instruction";
>   }
>
> +//===----------------------------------------------------------------------===//
> +// ReportUndefOperand.
> +
>   std::string ReportUndefOperand::getMessage() const {
>     return ("undef operand in branch at BB: " + BB->getName()).str();
>   }
>
> +//===----------------------------------------------------------------------===//
> +// ReportNonAffBranch.
> +
>   std::string ReportNonAffBranch::getMessage() const {
>     return ("Non affine branch in BB '" + BB->getName()).str() + "' with LHS: " +
>            *LHS + " and RHS: " + *RHS;
>   }
>
> +//===----------------------------------------------------------------------===//
> +// ReportNoBasePtr.
> +
>   std::string ReportNoBasePtr::getMessage() const { return "No base pointer"; }
>
> +//===----------------------------------------------------------------------===//
> +// ReportUndefBasePtr.
> +
>   std::string ReportUndefBasePtr::getMessage() const {
>     return "Undefined base pointer";
>   }
>
> +//===----------------------------------------------------------------------===//
> +// ReportVariantBasePtr.
> +
>   std::string ReportVariantBasePtr::getMessage() const {
>     return "Base address not invariant in current region:" + *BaseValue;
>   }
>
> +//===----------------------------------------------------------------------===//
> +// ReportNonAffineAccess.
> +
>   std::string ReportNonAffineAccess::getMessage() const {
>     return "Non affine access function: " + *AccessFunction;
>   }
>
> -ReportIndVar::ReportIndVar() { ++BadIndVarForScop; }
> +//===----------------------------------------------------------------------===//
> +// ReportIndVar.
> +
> +ReportIndVar::ReportIndVar() : RejectReason() { ++BadIndVarForScop; }
> +
> +//===----------------------------------------------------------------------===//
> +// ReportPhiNodeRefInRegion.
> +
> +ReportPhiNodeRefInRegion::ReportPhiNodeRefInRegion(Instruction *Inst)
> +    : ReportIndVar(), Inst(Inst) {}
>
>   std::string ReportPhiNodeRefInRegion::getMessage() const {
>     return "SCEV of PHI node refers to SSA names in region: " + *Inst;
>   }
>
> +const DebugLoc &ReportPhiNodeRefInRegion::getDebugLoc() const {
> +  return Inst->getDebugLoc();
> +}
> +
> +//===----------------------------------------------------------------------===//
> +// ReportNonCanonicalPhiNode.
> +
> +ReportNonCanonicalPhiNode::ReportNonCanonicalPhiNode(Instruction *Inst)
> +    : ReportIndVar(), Inst(Inst) {}
> +
>   std::string ReportNonCanonicalPhiNode::getMessage() const {
>     return "Non canonical PHI node: " + *Inst;
>   }
>
> +const DebugLoc &ReportNonCanonicalPhiNode::getDebugLoc() const {
> +  return Inst->getDebugLoc();
> +}
> +
> +//===----------------------------------------------------------------------===//
> +// ReportLoopHeader.
> +
> +ReportLoopHeader::ReportLoopHeader(Loop *L) : ReportIndVar(), L(L) {}
> +
>   std::string ReportLoopHeader::getMessage() const {
>     return ("No canonical IV at loop header: " + L->getHeader()->getName()).str();
>   }
>
> -ReportIndEdge::ReportIndEdge() { ++BadIndEdgeForScop; }
> +const DebugLoc &ReportLoopHeader::getDebugLoc() const {
> +  BasicBlock *BB = L->getHeader();
> +  return BB->getTerminator()->getDebugLoc();
> +}
> +
> +//===----------------------------------------------------------------------===//
> +// ReportIndEdge.
> +
> +ReportIndEdge::ReportIndEdge(BasicBlock *BB) : RejectReason(), BB(BB) {
> +  ++BadIndEdgeForScop;
> +}
>
>   std::string ReportIndEdge::getMessage() const {
>     return "Region has invalid entering edges!";
>   }
>
> +const DebugLoc &ReportIndEdge::getDebugLoc() const {
> +  return BB->getTerminator()->getDebugLoc();
> +}
> +
> +//===----------------------------------------------------------------------===//
> +// ReportLoopBound.
> +
>   ReportLoopBound::ReportLoopBound(Loop *L, const SCEV *LoopCount)
> -    : L(L), LoopCount(LoopCount) {
> +    : RejectReason(), L(L), LoopCount(LoopCount) {
>     ++BadLoopBoundForScop;
>   }
>
> @@ -158,7 +273,15 @@ std::string ReportLoopBound::getMessage() const {
>            L->getHeader()->getName();
>   }
>
> -ReportFuncCall::ReportFuncCall(Instruction *Inst) : Inst(Inst) {
> +const DebugLoc &ReportLoopBound::getDebugLoc() const {
> +  const BasicBlock *BB = L->getHeader();
> +  return BB->getTerminator()->getDebugLoc();
> +}
> +
> +//===----------------------------------------------------------------------===//
> +// ReportFuncCall.
> +
> +ReportFuncCall::ReportFuncCall(Instruction *Inst) : RejectReason(), Inst(Inst) {
>     ++BadFuncCallForScop;
>   }
>
> @@ -166,7 +289,22 @@ std::string ReportFuncCall::getMessage() const {
>     return "Call instruction: " + *Inst;
>   }
>
> -ReportAlias::ReportAlias(AliasSet *AS) : AS(AS) { ++BadAliasForScop; }
> +const DebugLoc &ReportFuncCall::getDebugLoc() const {
> +  return Inst->getDebugLoc();
> +}
> +
> +std::string ReportFuncCall::getEndUserMessage() const {
> +  return "This function call cannot be handeled. "
> +         "Try to inline it.";
> +}
> +
> +//===----------------------------------------------------------------------===//
> +// ReportAlias.
> +
> +ReportAlias::ReportAlias(Instruction *Inst, AliasSet *AS)
> +    : RejectReason(), AS(AS), Inst(Inst) {
> +  ++BadAliasForScop;
> +}
>
>   std::string ReportAlias::formatInvalidAlias(AliasSet &AS) const {
>     std::string Message;
> @@ -204,31 +342,89 @@ std::string ReportAlias::formatInvalidAlias(AliasSet &AS) const {
>
>   std::string ReportAlias::getMessage() const { return formatInvalidAlias(*AS); }
>
> -ReportSimpleLoop::ReportSimpleLoop() { ++BadSimpleLoopForScop; }
> +const DebugLoc &ReportAlias::getDebugLoc() const { return Inst->getDebugLoc(); }
> +
> +//===----------------------------------------------------------------------===//
> +// ReportSimpleLoop.
> +
> +ReportSimpleLoop::ReportSimpleLoop() : RejectReason() {
> +  ++BadSimpleLoopForScop;
> +}
>
>   std::string ReportSimpleLoop::getMessage() const {
>     return "Loop not in simplify form is invalid!";
>   }
>
> -ReportOther::ReportOther() { ++BadOtherForScop; }
> +//===----------------------------------------------------------------------===//
> +// ReportOther.
> +
> +std::string ReportOther::getMessage() const { return "Unknown reject reason"; }
> +
> +ReportOther::ReportOther() : RejectReason() { ++BadOtherForScop; }
> +
> +//===----------------------------------------------------------------------===//
> +// ReportIntToPtr.
> +ReportIntToPtr::ReportIntToPtr(Instruction *BaseValue)
> +    : ReportOther(), BaseValue(BaseValue) {}
>
>   std::string ReportIntToPtr::getMessage() const {
>     return "Find bad intToptr prt: " + *BaseValue;
>   }
>
> +const DebugLoc &ReportIntToPtr::getDebugLoc() const {
> +  return BaseValue->getDebugLoc();
> +}
> +
> +//===----------------------------------------------------------------------===//
> +// ReportAlloca.
> +
> +ReportAlloca::ReportAlloca(Instruction *Inst) : ReportOther(), Inst(Inst) {}
> +
>   std::string ReportAlloca::getMessage() const {
>     return "Alloca instruction: " + *Inst;
>   }
>
> +const DebugLoc &ReportAlloca::getDebugLoc() const {
> +  return Inst->getDebugLoc();
> +}
> +
> +//===----------------------------------------------------------------------===//
> +// ReportUnknownInst.
> +
> +ReportUnknownInst::ReportUnknownInst(Instruction *Inst)
> +    : ReportOther(), Inst(Inst) {}
> +
>   std::string ReportUnknownInst::getMessage() const {
>     return "Unknown instruction: " + *Inst;
>   }
>
> +const DebugLoc &ReportUnknownInst::getDebugLoc() const {
> +  return Inst->getDebugLoc();
> +}
> +
> +//===----------------------------------------------------------------------===//
> +// ReportPHIinExit.
> +
> +ReportPHIinExit::ReportPHIinExit(Instruction *Inst)
> +    : ReportOther(), Inst(Inst) {}
> +
>   std::string ReportPHIinExit::getMessage() const {
>     return "PHI node in exit BB";
>   }
>
> +const DebugLoc &ReportPHIinExit::getDebugLoc() const {
> +  return Inst->getDebugLoc();
> +}
> +
> +//===----------------------------------------------------------------------===//
> +// ReportEntry.
> +ReportEntry::ReportEntry(BasicBlock *BB) : ReportOther(), BB(BB) {}
> +
>   std::string ReportEntry::getMessage() const {
>     return "Region containing entry block of function is invalid!";
>   }
> +
> +const DebugLoc &ReportEntry::getDebugLoc() const {
> +  return BB->getTerminator()->getDebugLoc();
> +}
>   } // namespace polly
> diff --git a/test/ScopDetectionDiagnostics/ReportFuncCall-01.ll b/test/ScopDetectionDiagnostics/ReportFuncCall-01.ll
> new file mode 100644
> index 0000000..e87f072
> --- /dev/null
> +++ b/test/ScopDetectionDiagnostics/ReportFuncCall-01.ll
> @@ -0,0 +1,67 @@
> +; RUN: opt %loadPolly -pass-remarks="polly-detect" -polly-detect-track-failures -polly-detect -analyze < %s 2>&1 | FileCheck %s
> +
> +; #define N 1024
> +; double invalidCall(double A[N]);
> +;
> +; void a(double A[N], int n) {
> +;   for (int i=0; i<n; ++i) {
> +;     A[i] = invalidCall(A);
> +;   }
> +; }
> +
> +target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
> +target triple = "x86_64-unknown-linux-gnu"
> +
> +define void @a(double* %A, i32 %n) #0 {
> +entry:
> +  %cmp1 = icmp sgt i32 %n, 0, !dbg !10
> +  br i1 %cmp1, label %for.body.lr.ph, label %for.end, !dbg !10
> +
> +for.body.lr.ph:                                   ; preds = %entry
> +  %0 = zext i32 %n to i64
> +  br label %for.body, !dbg !10
> +
> +for.body:                                         ; preds = %for.body, %for.body.lr.ph
> +  %indvar = phi i64 [ 0, %for.body.lr.ph ], [ %indvar.next, %for.body ]
> +  %arrayidx = getelementptr double* %A, i64 %indvar, !dbg !12
> +  %call = tail call double @invalidCall(double* %A) #2, !dbg !12
> +  store double %call, double* %arrayidx, align 8, !dbg !12, !tbaa !14
> +  %indvar.next = add i64 %indvar, 1, !dbg !10
> +  %exitcond = icmp eq i64 %indvar.next, %0, !dbg !10
> +  br i1 %exitcond, label %for.end.loopexit, label %for.body, !dbg !10
> +
> +for.end.loopexit:                                 ; preds = %for.body
> +  br label %for.end
> +
> +for.end:                                          ; preds = %for.end.loopexit, %entry
> +  ret void, !dbg !18
> +}
> +
> +declare double @invalidCall(double*) #1
> +
> +; CHECK: remark: ReportFuncCall.c:4:8: The following errors keep this region from being a Scop.
> +; CHECK: remark: ReportFuncCall.c:5:12: This function call cannot be handeled. Try to inline it.
> +
> +!llvm.dbg.cu = !{!0}
> +!llvm.module.flags = !{!7, !8}
> +!llvm.ident = !{!9}
> +
> +!0 = metadata !{i32 786449, metadata !1, i32 12, metadata !"clang version 3.5.0 ", i1 true, metadata !"", i32 0, metadata !2, metadata !2, metadata !3, metadata !2, metadata !2, metadata !"", i32 2} ; [ DW_TAG_compile_unit ] [/home/simbuerg/Projekte/llvm/tools/polly/test/ScopDetectionDiagnostics/ReportFuncCall.c] [DW_LANG_C99]
> +!1 = metadata !{metadata !"ReportFuncCall.c", metadata !"/home/simbuerg/Projekte/llvm/tools/polly/test/ScopDetectionDiagnostics"}
> +!2 = metadata !{}
> +!3 = metadata !{metadata !4}
> +!4 = metadata !{i32 786478, metadata !1, metadata !5, metadata !"a", metadata !"a", metadata !"", i32 3, metadata !6, i1 false, i1 true, i32 0, i32 0, null, i32 256, i1 true, void (double*, i32)* @a, null, null, metadata !2, i32 3} ; [ DW_TAG_subprogram ] [line 3] [def] [a]
> +!5 = metadata !{i32 786473, metadata !1}          ; [ DW_TAG_file_type ] [/home/simbuerg/Projekte/llvm/tools/polly/test/ScopDetectionDiagnostics/ReportFuncCall.c]
> +!6 = metadata !{i32 786453, i32 0, null, metadata !"", i32 0, i64 0, i64 0, i64 0, i32 0, null, metadata !2, i32 0, null, null, null} ; [ DW_TAG_subroutine_type ] [line 0, size 0, align 0, offset 0] [from ]
> +!7 = metadata !{i32 2, metadata !"Dwarf Version", i32 4}
> +!8 = metadata !{i32 2, metadata !"Debug Info Version", i32 1}
> +!9 = metadata !{metadata !"clang version 3.5.0 "}
> +!10 = metadata !{i32 4, i32 8, metadata !11, null}
> +!11 = metadata !{i32 786443, metadata !1, metadata !4, i32 4, i32 3, i32 0, i32 0} ; [ DW_TAG_lexical_block ] [/home/simbuerg/Projekte/llvm/tools/polly/test/ScopDetectionDiagnostics/ReportFuncCall.c]
> +!12 = metadata !{i32 5, i32 12, metadata !13, null}
> +!13 = metadata !{i32 786443, metadata !1, metadata !11, i32 4, i32 27, i32 0, i32 1} ; [ DW_TAG_lexical_block ] [/home/simbuerg/Projekte/llvm/tools/polly/test/ScopDetectionDiagnostics/ReportFuncCall.c]
> +!14 = metadata !{metadata !15, metadata !15, i64 0}
> +!15 = metadata !{metadata !"double", metadata !16, i64 0}
> +!16 = metadata !{metadata !"omnipotent char", metadata !17, i64 0}
> +!17 = metadata !{metadata !"Simple C/C++ TBAA"}
> +!18 = metadata !{i32 7, i32 1, metadata !4, null}
>



More information about the llvm-commits mailing list