[PATCH] D33163: [Polly] Added the list of Instructions to output in ScopInfo pass
Tobias Grosser via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu May 25 21:50:03 PDT 2017
grosser added a comment.
Hi Nandini,
thanks for the update. Find below some more comments.
I also saw that with your patch we currently get 118 unexpected failures when running 'make check-polly'. They seem to be caused because we print instructions now always. I think this might be too verbose in general. Can we make instruction printing optional and hide it behind a flag?
================
Comment at: include/polly/ScopInfo.h:1145
+ ScopStmt(Scop &parent, BasicBlock &bb, Loop *SurroundingLoop,
+ std::vector<Instruction *> bbInst);
----------------
In LLVM Variables start with uppercase letters.
Also, I would just call it "Instructions"
================
Comment at: include/polly/ScopInfo.h:1253
+ /// Vector for Instructions in a BB.
+ std::vector<Instruction *> bbInst;
+
----------------
Please use uppercase.
================
Comment at: include/polly/ScopInfo.h:1540
+ /// Print the instructions in ScopStmt.
+ ///
+ void printInstructions(raw_ostream &OS) const;
----------------
Add an empty line between print() and the comment that follows it.
================
Comment at: include/polly/ScopInfo.h:2030
+ void addScopStmt(BasicBlock *BB, Loop *SurroundingLoop,
+ std::vector<Instruction *> bbInst);
----------------
Uppercase, variable name.
================
Comment at: lib/Analysis/ScopBuilder.cpp:635
else {
+ std::vector<Instruction *> bbInst;
+ for (Instruction &Inst : *I->getNodeAs<BasicBlock>())
----------------
Uppercase, variable name.
================
Comment at: lib/Analysis/ScopInfo.cpp:1654
+ R(nullptr), Build(nullptr), SurroundingLoop(SurroundingLoop),
+ bbInst(bbInst) {
----------------
Uppercase variable name.
================
Comment at: lib/Analysis/ScopInfo.cpp:4663
assert(BB && "Unexpected nullptr!");
- Stmts.emplace_back(*this, *BB, SurroundingLoop);
+ Stmts.emplace_back(*this, *BB, SurroundingLoop, bbInst);
auto *Stmt = &Stmts.back();
----------------
Uppercase variable name.
================
Comment at: test/ScopInfo/statement.ll:15
+; CHECK-NEXT: br i1 %cmp, label %for.body, label %for.end
+; CHECK-NEXT: br label %Stmt
+; CHECK-NEXT: %idxprom = sext i32 %i.0 to i64
----------------
These CHECK lines do not seem correct. If I run
```
po -polly-process-unprofitable -polly-remarks-minimal -polly-use-llvm-names -polly-scops -analyze < /home/grosser/Projects/polly/git/tools/polly/test/ScopInfo/statement.ll
```
I get:
```
Statements {
Stmt_Stmt
Domain :=
{ Stmt_Stmt[i0] : 0 <= i0 <= 1023 };
Schedule :=
{ Stmt_Stmt[i0] -> [i0] };
MustWriteAccess := [Reduction Type: NONE] [Scalar: 0]
{ Stmt_Stmt[i0] -> MemRef_A[i0] };
MustWriteAccess := [Reduction Type: NONE] [Scalar: 0]
{ Stmt_Stmt[i0] -> MemRef_B[i0] };
Instructions {
%idxprom = sext i32 %i.0 to i64
%arrayidx = getelementptr inbounds i32, i32* %A, i64 %idxprom
store i32 %i.0, i32* %arrayidx, align 4
%idxprom1 = sext i32 %i.0 to i64
%arrayidx2 = getelementptr inbounds i32, i32* %B, i64 %idxprom1
store i32 %i.0, i32* %arrayidx2, align 4
br label %for.inc
}
}
```
These four "CHECK" lines do not make sense:
```
; CHECK-NEXT: %i.0 = phi i32 [ 0, %entry ], [ %add, %for.inc ]
; CHECK-NEXT: %cmp = icmp slt i32 %i.0, 1024
; CHECK-NEXT: br i1 %cmp, label %for.body, label %for.end
; CHECK-NEXT: br label %Stmt
```
Also, the indentation seems off.
https://reviews.llvm.org/D33163
More information about the llvm-commits
mailing list