[PATCH] D123748: [ValueTracking] Added support to deduce PHI Nodes values being a power of 2

[William Junda Huang via Phabricator via llvm-commits]

huangjd added inline comments.


================
Comment at: llvm/lib/Analysis/ValueTracking.cpp:2028
+/// always a power of two (or zero).
+static bool isPowerOfTwoRecurrence(const PHINode *PN, bool OrZero,
+                                   unsigned Depth, Query &Q) {
----------------
Original comment from davidxl:
run a clang-format on the changed part.



================
Comment at: llvm/lib/Analysis/ValueTracking.cpp:2044
+
+  // Except for Mul, the induction variable must be on the left side of the
+  // increment expression, otherwise its value can be arbitrary.
----------------
Original comment from davidxl:
typo: induction




================
Comment at: llvm/test/Analysis/ValueTracking/known-power-of-two-urem.ll:2
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt -S -instcombine < %s | FileCheck %s
+
----------------
Original comment from xbolva00
Please use utils/update_test_checks.py




================
Comment at: llvm/test/Analysis/ValueTracking/known-power-of-two-urem.ll:20
+
+define i64 @known_power_of_two_urem_phi(i64 %size, i1 %cmp, i1 %cmp1) {
+; CHECK-LABEL: @known_power_of_two_urem_phi(
----------------
Original comment from davidxl:

Please also add tests for

1. other operators for simple recurrence.
2. for other type of recurrence (change at line 2177).
3. chained phi case
4. negative cases.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123748