[PATCH] D12053: [Polly] Workaround for SDiv/SRem referenced from SCEVExpander

[Michael Kruse via llvm-commits]

Meinersbur added a comment.

Note that I I hope this is a temporary workaround and we can get LLVM to implement SCEVSRemExpr and SCEVSDivExpr.


================
Comment at: lib/CodeGen/IslNodeBuilder.cpp:58
@@ -53,1 +57,3 @@
+}
+
 __isl_give isl_ast_expr *
----------------
jdoerfert wrote:
> Why do we need this?
LLVM defines SDivOperator, but not SRemOperator

================
Comment at: lib/CodeGen/IslNodeBuilder.cpp:724
@@ +723,3 @@
+      if (isa<SDivOperator>(Inst) || isa<SRemOperator>(Inst))
+        Worklist.push_back(&cast<Instruction>(Inst));
+    }
----------------
jdoerfert wrote:
> What type does Inst have if not Instruction &?
The worklist push below is of type llvm::User*, but it might indeed be unnecessary here (and below since the cast has already been done)

================
Comment at: lib/CodeGen/IslNodeBuilder.cpp:731
@@ +730,3 @@
+    if (ValueReplacer.count(Item))
+      continue; // Already processed
+
----------------
jdoerfert wrote:
> How can we look at an instruction twice (if we always copy the instruction as noted below)?
If used in operands of two instructions?!? I obviously don't understand your question.

================
Comment at: lib/CodeGen/IslNodeBuilder.cpp:737
@@ +736,3 @@
+      continue;
+
+    bool DepWithinRegion = false;
----------------
jdoerfert wrote:
> Can we just copy it without all the checks if we can move it?
The move in the comment is outdated. I tried to move it in an previous implementation and found that moving doesn't come without problems.

There is still nothing to do if SCEVValidator rejected it. 


http://reviews.llvm.org/D12053