[PATCH] [Polly] Model PHI nodes without demoting them
Tobias Grosser
tobias at grosser.es
Wed Feb 4 23:20:18 PST 2015
Hi Johannes,
the patch looks good in general. As you mentioned earlier, the test coverage is still a little low. I would be interested in a loop with a non-IV PHI node, as well as two tests where the values in the PHis are instructions, once executed in the incoming basic blocks and once in a block that is post-dominated by the relevant incoming block. What do you think?
Cheers,
Tobias
================
Comment at: lib/Analysis/ScopDetection.cpp:155
@@ +154,3 @@
+ cl::desc(
+ "Allow PHI nodes in the input [Unsafe with any code-generation!]."),
+ cl::location(PollyModelPHINodes), cl::Hidden, cl::ZeroOrMore,
----------------
without
================
Comment at: lib/Analysis/TempScopInfo.cpp:102
@@ +101,3 @@
+void TempScopInfo::buildPHIAccesses(PHINode *PHI, Region &R,
+ AccFuncSetType &Functions) {
+ if (canSynthesize(PHI, LI, SE, &R))
----------------
Maybe a comment on how we handle PHI accesses, where loads/stores are created.
================
Comment at: lib/Analysis/TempScopInfo.cpp:111
@@ +110,3 @@
+ if (!R.contains(OpBB))
+ continue;
+
----------------
Can this ever happen? If it can, maybe this is worth a test case, otherwise maybe make this an assert?
================
Comment at: lib/Analysis/TempScopInfo.cpp:229
@@ -200,1 +228,3 @@
+ }
+
if (!isa<StoreInst>(Inst) && buildScalarDependences(Inst, &R)) {
----------------
No need for braces?
http://reviews.llvm.org/D7415
EMAIL PREFERENCES
http://reviews.llvm.org/settings/panel/emailpreferences/
More information about the llvm-commits
mailing list