[polly] r267559 - Allow unsigned comparisons

Tobias Grosser via llvm-commits llvm-commits at lists.llvm.org
Tue Apr 26 22:46:42 PDT 2016


On 04/26/2016 04:33 PM, Johannes Doerfert via llvm-commits wrote:
> Author: jdoerfert
> Date: Tue Apr 26 09:33:12 2016
> New Revision: 267559
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=267559&view=rev
> Log:
> Allow unsigned comparisons
> 
>   With this patch we will optimistically assume that the result of an unsigned
>   comparison is the same as the result of the same comparison interpreted as
>   signed.

After this commit, I see a miscompile in
http://lab.llvm.org:8011/builders/perf-x86_64-penryn-O3-polly-before-vectorizer-unprofitable/builds/1716

Best,
Tobias


> 
> Added:
>     polly/trunk/test/ScopInfo/simple_loop_unsigned.ll
>     polly/trunk/test/ScopInfo/simple_loop_unsigned_2.ll
>     polly/trunk/test/ScopInfo/simple_loop_unsigned_3.ll
> Modified:
>     polly/trunk/include/polly/ScopDetectionDiagnostic.h
>     polly/trunk/lib/Analysis/ScopDetection.cpp
>     polly/trunk/lib/Analysis/ScopDetectionDiagnostic.cpp
>     polly/trunk/lib/Analysis/ScopInfo.cpp
>     polly/trunk/test/ScopInfo/unsigned-condition.ll
> 
> Modified: polly/trunk/include/polly/ScopDetectionDiagnostic.h
> URL: http://llvm.org/viewvc/llvm-project/polly/trunk/include/polly/ScopDetectionDiagnostic.h?rev=267559&r1=267558&r2=267559&view=diff
> ==============================================================================
> --- polly/trunk/include/polly/ScopDetectionDiagnostic.h (original)
> +++ polly/trunk/include/polly/ScopDetectionDiagnostic.h Tue Apr 26 09:33:12 2016
> @@ -68,7 +68,6 @@ enum RejectReasonKind {
>    rrkAffFunc,
>    rrkUndefCond,
>    rrkInvalidCond,
> -  rrkUnsignedCond,
>    rrkUndefOperand,
>    rrkNonAffBranch,
>    rrkNoBasePtr,
> @@ -345,32 +344,6 @@ public:
>    //@}
>  };
>  
> -//===----------------------------------------------------------------------===//
> -/// @brief Captures an condition on unsigned values
> -///
> -/// We do not yet allow conditions on unsigend values
> -class ReportUnsignedCond : public ReportAffFunc {
> -  //===--------------------------------------------------------------------===//
> -
> -  // The BasicBlock we found the broken condition in.
> -  BasicBlock *BB;
> -
> -public:
> -  ReportUnsignedCond(const Instruction *Inst, BasicBlock *BB)
> -      : ReportAffFunc(rrkUnsignedCond, Inst), BB(BB) {}
> -
> -  /// @name LLVM-RTTI interface
> -  //@{
> -  static bool classof(const RejectReason *RR);
> -  //@}
> -
> -  /// @name RejectReason interface
> -  //@{
> -  virtual std::string getMessage() const override;
> -  virtual std::string getEndUserMessage() const override;
> -  //@}
> -};
> -
>  //===----------------------------------------------------------------------===//
>  /// @brief Captures an undefined operand.
>  class ReportUndefOperand : public ReportAffFunc {
> 
> Modified: polly/trunk/lib/Analysis/ScopDetection.cpp
> URL: http://llvm.org/viewvc/llvm-project/polly/trunk/lib/Analysis/ScopDetection.cpp?rev=267559&r1=267558&r2=267559&view=diff
> ==============================================================================
> --- polly/trunk/lib/Analysis/ScopDetection.cpp (original)
> +++ polly/trunk/lib/Analysis/ScopDetection.cpp Tue Apr 26 09:33:12 2016
> @@ -149,11 +149,6 @@ static cl::opt<bool>
>                             cl::Hidden, cl::init(false), cl::ZeroOrMore,
>                             cl::cat(PollyCategory));
>  
> -static cl::opt<bool> AllowUnsigned("polly-allow-unsigned",
> -                                   cl::desc("Allow unsigned expressions"),
> -                                   cl::Hidden, cl::init(false), cl::ZeroOrMore,
> -                                   cl::cat(PollyCategory));
> -
>  static cl::opt<bool, true>
>      TrackFailures("polly-detect-track-failures",
>                    cl::desc("Track failure strings in detecting scop regions"),
> @@ -385,13 +380,6 @@ bool ScopDetection::isValidBranch(BasicB
>    }
>  
>    ICmpInst *ICmp = cast<ICmpInst>(Condition);
> -  // Unsigned comparisons are not allowed. They trigger overflow problems
> -  // in the code generation.
> -  //
> -  // TODO: This is not sufficient and just hides bugs. However it does pretty
> -  //       well.
> -  if (ICmp->isUnsigned() && !AllowUnsigned)
> -    return invalid<ReportUnsignedCond>(Context, /*Assert=*/true, BI, &BB);
>  
>    // Are both operands of the ICmp affine?
>    if (isa<UndefValue>(ICmp->getOperand(0)) ||
> 
> Modified: polly/trunk/lib/Analysis/ScopDetectionDiagnostic.cpp
> URL: http://llvm.org/viewvc/llvm-project/polly/trunk/lib/Analysis/ScopDetectionDiagnostic.cpp?rev=267559&r1=267558&r2=267559&view=diff
> ==============================================================================
> --- polly/trunk/lib/Analysis/ScopDetectionDiagnostic.cpp (original)
> +++ polly/trunk/lib/Analysis/ScopDetectionDiagnostic.cpp Tue Apr 26 09:33:12 2016
> @@ -196,22 +196,6 @@ bool ReportInvalidCond::classof(const Re
>  }
>  
>  //===----------------------------------------------------------------------===//
> -// ReportUnsignedCond.
> -
> -std::string ReportUnsignedCond::getMessage() const {
> -  return ("Condition in BB '" + BB->getName()).str() +
> -         "' performs a comparision on (not yet supported) unsigned integers.";
> -}
> -
> -std::string ReportUnsignedCond::getEndUserMessage() const {
> -  return "Unsupported comparision on unsigned integers encountered";
> -}
> -
> -bool ReportUnsignedCond::classof(const RejectReason *RR) {
> -  return RR->getKind() == rrkUnsignedCond;
> -}
> -
> -//===----------------------------------------------------------------------===//
>  // ReportUndefOperand.
>  
>  std::string ReportUndefOperand::getMessage() const {
> 
> Modified: polly/trunk/lib/Analysis/ScopInfo.cpp
> URL: http://llvm.org/viewvc/llvm-project/polly/trunk/lib/Analysis/ScopInfo.cpp?rev=267559&r1=267558&r2=267559&view=diff
> ==============================================================================
> --- polly/trunk/lib/Analysis/ScopInfo.cpp (original)
> +++ polly/trunk/lib/Analysis/ScopInfo.cpp Tue Apr 26 09:33:12 2016
> @@ -1317,6 +1317,18 @@ buildConditionSets(ScopStmt &Stmt, Value
>      isl_pw_aff *LHS, *RHS;
>      LHS = Stmt.getPwAff(SE.getSCEVAtScope(ICond->getOperand(0), L));
>      RHS = Stmt.getPwAff(SE.getSCEVAtScope(ICond->getOperand(1), L));
> +
> +    if (ICond->isUnsigned()) {
> +      // For unsigned comparisons we assumed the signed bit of neither operand
> +      // to be set. The comparison is equal to a signed comparison under this
> +      // assumption.
> +      auto *BB = Stmt.getEntryBlock();
> +      S.recordAssumption(UNSIGNED, isl_pw_aff_nonneg_set(isl_pw_aff_copy(LHS)),
> +                         TI->getDebugLoc(), AS_ASSUMPTION, BB);
> +      S.recordAssumption(UNSIGNED, isl_pw_aff_nonneg_set(isl_pw_aff_copy(RHS)),
> +                         TI->getDebugLoc(), AS_ASSUMPTION, BB);
> +    }
> +
>      ConsequenceCondSet =
>          buildConditionSet(ICond->getPredicate(), LHS, RHS, Domain);
>    }
> 
> Added: polly/trunk/test/ScopInfo/simple_loop_unsigned.ll
> URL: http://llvm.org/viewvc/llvm-project/polly/trunk/test/ScopInfo/simple_loop_unsigned.ll?rev=267559&view=auto
> ==============================================================================
> --- polly/trunk/test/ScopInfo/simple_loop_unsigned.ll (added)
> +++ polly/trunk/test/ScopInfo/simple_loop_unsigned.ll Tue Apr 26 09:33:12 2016
> @@ -0,0 +1,32 @@
> +; RUN: opt %loadPolly -polly-scops -analyze < %s | FileCheck %s
> +
> +; void f(int a[], unsigned N) {
> +;   unsigned i;
> +;   do {
> +;     a[i] = i;
> +;   } while (++i <= N);
> +; }
> +
> +; CHECK:      Assumed Context:
> +; CHECK-NEXT: [N] -> {  : N >= 0 }
> +;
> +; CHECK:              Domain :=
> +; CHECK-NEXT:             [N] -> { Stmt_bb[i0] : 0 <= i0 < N; Stmt_bb[0] : N <= 0 };
> +
> +target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128"
> +
> +define void @f(i64* nocapture %a, i64 %N) nounwind {
> +entry:
> +  br label %bb
> +
> +bb:                                               ; preds = %bb, %entry
> +  %i = phi i64 [ 0, %entry ], [ %i.inc, %bb ]
> +  %scevgep = getelementptr inbounds i64, i64* %a, i64 %i
> +  store i64 %i, i64* %scevgep
> +  %i.inc = add nsw i64 %i, 1
> +  %exitcond = icmp uge i64 %i.inc, %N
> +  br i1 %exitcond, label %return, label %bb
> +
> +return:                                           ; preds = %bb, %entry
> +  ret void
> +}
> 
> Added: polly/trunk/test/ScopInfo/simple_loop_unsigned_2.ll
> URL: http://llvm.org/viewvc/llvm-project/polly/trunk/test/ScopInfo/simple_loop_unsigned_2.ll?rev=267559&view=auto
> ==============================================================================
> --- polly/trunk/test/ScopInfo/simple_loop_unsigned_2.ll (added)
> +++ polly/trunk/test/ScopInfo/simple_loop_unsigned_2.ll Tue Apr 26 09:33:12 2016
> @@ -0,0 +1,25 @@
> +; RUN: opt %loadPolly -polly-scops -analyze < %s | FileCheck %s
> +
> +; CHECK:      Assumed Context:
> +; CHECK-NEXT: [N] -> {  : N > 0 }
> +;
> +; CHECK:              Domain :=
> +; CHECK-NEXT:             [N] -> { Stmt_bb[i0] : 0 <= i0 < N; Stmt_bb[0] : N <= 0 };
> +;
> +target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128"
> +
> +define void @f(i64* nocapture %a, i64 %N) nounwind {
> +entry:
> +  br label %bb
> +
> +bb:                                               ; preds = %bb, %entry
> +  %i = phi i64 [ %N, %entry ], [ %i.dec, %bb ]
> +  %scevgep = getelementptr inbounds i64, i64* %a, i64 %i
> +  store i64 %i, i64* %scevgep
> +  %i.dec = add nsw i64 %i, -1
> +  %exitcond = icmp ugt i64 %i.dec, 0
> +  br i1 %exitcond, label %bb, label %return
> +
> +return:                                           ; preds = %bb, %entry
> +  ret void
> +}
> 
> Added: polly/trunk/test/ScopInfo/simple_loop_unsigned_3.ll
> URL: http://llvm.org/viewvc/llvm-project/polly/trunk/test/ScopInfo/simple_loop_unsigned_3.ll?rev=267559&view=auto
> ==============================================================================
> --- polly/trunk/test/ScopInfo/simple_loop_unsigned_3.ll (added)
> +++ polly/trunk/test/ScopInfo/simple_loop_unsigned_3.ll Tue Apr 26 09:33:12 2016
> @@ -0,0 +1,26 @@
> +; RUN: opt %loadPolly -polly-scops -analyze < %s | FileCheck %s
> +
> +; CHECK:      Assumed Context:
> +; CHECK-NEXT: [N] -> { : }
> +;
> +; CHECK:              Domain :=
> +; CHECK-NEXT:             [N] -> { Stmt_bb[i0] : 0 < i0 <= 1000 - N; Stmt_bb[0] };
> +
> +target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128"
> +
> +define void @f(i64* nocapture %a, i64 %N) nounwind {
> +entry:
> +  br label %bb
> +
> +bb:                                               ; preds = %bb, %entry
> +  %i = phi i64 [ 1000, %entry ], [ %i.dec, %bb ]
> +  %scevgep = getelementptr inbounds i64, i64* %a, i64 %i
> +  store i64 %i, i64* %scevgep
> +  %i.dec = add nsw i64 %i, -1
> +  %sub = sub nsw i64 %N, %i
> +  %exitcond = icmp ult i64 %sub, 0
> +  br i1 %exitcond, label %bb, label %return
> +
> +return:                                           ; preds = %bb, %entry
> +  ret void
> +}
> 
> Modified: polly/trunk/test/ScopInfo/unsigned-condition.ll
> URL: http://llvm.org/viewvc/llvm-project/polly/trunk/test/ScopInfo/unsigned-condition.ll?rev=267559&r1=267558&r2=267559&view=diff
> ==============================================================================
> --- polly/trunk/test/ScopInfo/unsigned-condition.ll (original)
> +++ polly/trunk/test/ScopInfo/unsigned-condition.ll Tue Apr 26 09:33:12 2016
> @@ -1,4 +1,4 @@
> -; RUN: opt %loadPolly -polly-scops -analyze -polly-allow-unsigned < %s | FileCheck %s
> +; RUN: opt %loadPolly -polly-scops -analyze < %s | FileCheck %s
>  
>  ; void f(int a[], int N, unsigned P) {
>  ;   int i;
> @@ -7,6 +7,17 @@
>  ;       a[i] = i;
>  ; }
>  
> +; The assumed context is the universe because the "signed-unsigned assumption"
> +;   [P, N] -> {  : N > 0 and P >= 0 }
> +; is implied by the execution domain. Thus if something is executed this
> +; assumption will hold.
> +
> +; CHECK:      Assumed Context:
> +; CHECK-NEXT: [P, N] -> {  :  }
> +;
> +; CHECK:              Domain :=
> +; CHECK-NEXT:             [P, N] -> { Stmt_store[i0] : P >= 42 and 0 <= i0 < N };
> +
>  target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128"
>  
>  define void @f(i64* nocapture %a, i64 %N, i64 %P) nounwind {
> @@ -31,17 +42,3 @@ bb.backedge:
>  return:
>    ret void
>  }
> -
> -
> -; CHECK:      Assumed Context:
> -; CHECK-NEXT: [P, N] -> {  :  }
> -;
> -; CHECK:      Statements {
> -; CHECK-NEXT:     Stmt_store
> -; CHECK-NEXT:         Domain :=
> -; CHECK-NEXT:             [P, N] -> { Stmt_store[i0] : P >= 42 and 0 <= i0 < N };
> -; CHECK-NEXT:         Schedule :=
> -; CHECK-NEXT:             [P, N] -> { Stmt_store[i0] -> [i0] };
> -; CHECK-NEXT:         MustWriteAccess :=    [Reduction Type: NONE] [Scalar: 0]
> -; CHECK-NEXT:             [P, N] -> { Stmt_store[i0] -> MemRef_a[i0] };
> -; CHECK-NEXT: }
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
> 



More information about the llvm-commits mailing list