[PATCH] D40369: Support sext instruction in SCEV delinearization algorithm (new revision)

[Dave Green via Phabricator via llvm-commits]

dmgreen added a comment.

Hello. I just have some test Nits.



================
Comment at: lib/Analysis/ScalarEvolution.cpp:10716
 static inline int numberOfTerms(const SCEV *S) {
+  if (const SCEVSignExtendExpr *SExtS = dyn_cast<SCEVSignExtendExpr>(S)) {
+    return numberOfTerms(SExtS->getOperand());
----------------
Don't need the brackets, like the if below


================
Comment at: test/Analysis/Delinearization/test_sext.ll:1
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt -O3 -delinearize -analyze < %s | FileCheck %s
----------------
This line doesn't seem to be true and can be removed I think


================
Comment at: test/Analysis/Delinearization/test_sext.ll:2
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt -O3 -delinearize -analyze < %s | FileCheck %s
+
----------------
It's probably best to not run -O3 here, as it could change the output as llvm evolves. You can run -O3 on the input now and use that generated IR for this test, then just needing -delinearize -analyze.


================
Comment at: test/Analysis/Delinearization/test_sext.ll:61
+; ModuleID = 'test_sext.c'
+source_filename = "test_sext.c"
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
----------------
source_filename and ModuleID can be removed


================
Comment at: test/Analysis/Delinearization/test_sext.ll:65
+
+; Function Attrs: nounwind uwtable
+define void @Test(i32* %a, i32 %N) #0 {
----------------
This Function Attrs comment and the #0 on the function can be removed


Repository:
  rL LLVM

https://reviews.llvm.org/D40369