[PATCH] D141850: [SCEV] Preserve divisibility and min/max information in applyLoopGuards

Mon Jan 23 01:58:58 PST 2023

caojoshua added a comment.

  After this patch, depending on the assume processing order applyLoopGuards could create the following SCEV:
  max(min((8 * (TC / 8)) , 96), 8)

This example looks wrong. I think min/max should be switched. Should be

  min(max((8 * (TC / 8)), 96), 8)

Please update description.

---

In terms of overall approach, I'm not sure. Feels a bit hacky to have custom logic to check that an expressions is a min/max of mul/div. I'll let others chime in here.

================
Comment at: llvm/lib/Analysis/ScalarEvolution.cpp:15121
+    // divisibilty information expressed as (A /u B) * B, while B is a constant.
+    // Return wether the divisibilty info was found, and the divisor B in \p
+    // DividesBy.
----------------
typo: wether

I'm not sure what `the divisor B in \p DividesBy` means.

I think this paragraph needs to be more clear. What does it mean to be composed on Min/Max SCEVs?

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

https://reviews.llvm.org/D141850