[PATCH] D114650: [SCEV] Construct SCEV iteratively.

[Florian Hahn via Phabricator via llvm-commits]

fhahn marked 2 inline comments as done.
fhahn added inline comments.


================
Comment at: llvm/lib/Analysis/ScalarEvolution.cpp:7264
+    Ops.push_back(BO->LHS);
+    Ops.push_back(BO->RHS);
+    return nullptr;
----------------
nikic wrote:
> Add and mul currently have special code that tries to combine multiple sequential adds/mul into one getAddExpr/getMulExpr call. This is effectively lost here (or maybe worse, we end up doing both -- each add individually here, and then a multiple-add variant in getSCEV).
> 
> I think if we're going to change this (which might well make sense), we should probably change that separately, and also in the createSCEV() code as well. I suspect that this might account for both some of the codegen changes and some of the compile-time impact.
Thanks, I updated `getOperandsToCreate` to also traverse mul/add chains. This indeed removed the `IndVarSimplify` changes and improved compile-time a bit.

We could go even further and enqueue (Opcode, Operands) tuples to create directly, instead of going through `createSCEV`, but this would require a way to encode at least negations for the Add case.



================
Comment at: llvm/lib/Analysis/ScalarEvolution.cpp:7290
+  case Instruction::GetElementPtr:
+    if (cast<GEPOperator>(U)->getSourceElementType()->isSized()) {
+      for (Value *Index : U->operands())
----------------
nikic wrote:
> https://github.com/llvm/llvm-project/commit/327307d9d4da0045f762f75343fe66b0f10ecc63 ;)
Replace with an assert


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D114650/new/

https://reviews.llvm.org/D114650