[PATCH] D112618: [BasicAA] Remove misleading overflow check

[Nikita Popov via Phabricator via llvm-commits]

nikic created this revision.
nikic added reviewers: fhahn, reames, courbet, jdoerfert.
Herald added subscribers: jeroen.dobbelaere, hiraditya.
nikic requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

GEP decomposition currently checks whether the multiplication of the linear expression offset and GEP scale overflows. However, if everything else works correctly, this overflow check is both unnecessary and dangerously misleading. While it will avoid an overflow in `Scale * Offset` in particular, other parts of the calculation (including those on dynamic values) may still overflow. The code working on the decomposed GEPs is responsible for ensuring that it remains correct in the presence of overflow. D112611 <https://reviews.llvm.org/D112611> fixes the last issue of that kind that I'm aware of (in fact, the overflow check was originally introduced to work around precisely that issue).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D112618

Files:
  llvm/lib/Analysis/BasicAliasAnalysis.cpp


Index: llvm/lib/Analysis/BasicAliasAnalysis.cpp
===================================================================
--- llvm/lib/Analysis/BasicAliasAnalysis.cpp
+++ llvm/lib/Analysis/BasicAliasAnalysis.cpp
@@ -657,23 +657,8 @@
 
       // The GEP index scale ("Scale") scales C1*V+C2, yielding (C1*V+C2)*Scale.
       // This gives us an aggregate computation of (C1*Scale)*V + C2*Scale.
-
-      // It can be the case that, even through C1*V+C2 does not overflow for
-      // relevant values of V, (C2*Scale) can overflow. In that case, we cannot
-      // decompose the expression in this way.
-      //
-      // FIXME: C1*Scale and the other operations in the decomposed
-      // (C1*Scale)*V+C2*Scale can also overflow. We should check for this
-      // possibility.
-      bool Overflow;
-      APInt ScaledOffset = LE.Offset.sextOrTrunc(MaxPointerSize)
-                           .smul_ov(Scale, Overflow);
-      if (Overflow) {
-        LE = LinearExpression(CastedValue(Index, 0, SExtBits, TruncBits));
-      } else {
-        Decomposed.Offset += ScaledOffset;
-        Scale *= LE.Scale.sextOrTrunc(MaxPointerSize);
-      }
+      Decomposed.Offset += LE.Offset.sextOrTrunc(MaxPointerSize) * Scale;
+      Scale *= LE.Scale.sextOrTrunc(MaxPointerSize);
 
       // If we already had an occurrence of this index variable, merge this
       // scale into it.  For example, we want to handle:


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D112618.382637.patch
Type: text/x-patch
Size: 1408 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20211027/8e7c2ae6/attachment.bin>