[PATCH] D104806: [ScalarEvolution] Make getMinusSCEV() fail for unrelated pointers.

Eli Friedman via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Jun 23 11:48:15 PDT 2021


efriedma created this revision.
efriedma added reviewers: reames, mkazantsev, nikic, fhahn, lebedev.ri.
Herald added subscribers: kerbowa, pengfei, javed.absar, hiraditya, nhaehnle, jvesely, nemanjai.
efriedma requested review of this revision.
Herald added a project: LLVM.

As part of making ScalarEvolution's handling of pointers consistent, we want to forbid multiplying a pointer by -1 (or any other value). This means we can't blindly subtract pointers.

There are a few ways we could deal with this:

1. We could completely forbid subtracting pointers in getMinusSCEV()
2. We could forbid subracting pointers with different pointer bases (this patch).
3. We could try to ptrtoint pointer operands.

The option in this patch is more friendly to non-integral pointers: code that works with normal pointers will also work with non-integral pointers. And it seems like there are very few places that actually benefit from the third option.

As a minimal patch, the ScalarEvolution implementation of getMinusSCEV still ends up subtracting pointers if they have the same base.  This should eliminate the shared pointer base, but eventually we'll need to rewrite it to avoid negating the pointer base. I plan to do this as a separate step to allow measuring the compile-time impact.

This doesn't cause obvious functional changes in most cases; the one case that is significantly affected is ICmpZero handling in LSR (which is the source of almost all the test changes).  The resulting changes seem okay to me, but suggestions welcome.  As an alternative, I tried explicitly ptrtoint'ing the operands, but the result doesn't seem obviously better.

I deleted the test lsr-undef-in-binop.ll becuase I couldn't figure out how to repair it to test what it was actually trying to test.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D104806

Files:
  llvm/include/llvm/Analysis/ScalarEvolution.h
  llvm/lib/Analysis/ScalarEvolution.cpp
  llvm/lib/Analysis/ScalarEvolutionAliasAnalysis.cpp
  llvm/lib/Analysis/StackSafetyAnalysis.cpp
  llvm/lib/Transforms/Scalar/AlignmentFromAssumptions.cpp
  llvm/lib/Transforms/Scalar/LoopRerollPass.cpp
  llvm/lib/Transforms/Scalar/LoopStrengthReduce.cpp
  llvm/test/Analysis/StackSafetyAnalysis/local.ll
  llvm/test/CodeGen/ARM/lsr-undef-in-binop.ll
  llvm/test/CodeGen/ARM/test-sharedidx.ll
  llvm/test/CodeGen/PowerPC/pr42492.ll
  llvm/test/CodeGen/X86/update-terminator-debugloc.ll
  llvm/test/Transforms/LoopStrengthReduce/AMDGPU/lsr-postinc-pos-addrspace.ll
  llvm/test/Transforms/LoopStrengthReduce/X86/eh-insertion-point-2.ll
  llvm/test/Transforms/LoopStrengthReduce/X86/eh-insertion-point.ll
  llvm/test/Transforms/LoopStrengthReduce/X86/expander-crashes.ll
  llvm/test/Transforms/LoopStrengthReduce/X86/expander-reused-value-insert-point.ll
  llvm/test/Transforms/LoopStrengthReduce/X86/ivchain-X86.ll
  llvm/test/Transforms/LoopStrengthReduce/funclet.ll
  llvm/test/Transforms/LoopStrengthReduce/pr27056.ll

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D104806.354041.patch
Type: text/x-patch
Size: 41088 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210623/4553ca11/attachment-0001.bin>


More information about the llvm-commits mailing list