[llvm] [Analysis] Bail out for negative offsets in isDereferenceableAndAlignedInLoop (PR #99490)

[via llvm-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-llvm-analysis

Author: David Sherwood (david-arm)

<details>
<summary>Changes</summary>

This patch is effectively NFC because it doesn't actually affect any existing tests. It's not possible to even write a test for it, because by accident negative offsets are caught by a subsequent unsigned add overflow check. However, I think it's better to bail out explicitly for negative offsets so that it's more consistent with the unsigned remainder and add calculations.

---
Full diff: https://github.com/llvm/llvm-project/pull/99490.diff


1 Files Affected:

- (modified) llvm/lib/Analysis/Loads.cpp (+7) 


``````````diff
diff --git a/llvm/lib/Analysis/Loads.cpp b/llvm/lib/Analysis/Loads.cpp
index 1d54a66705a2a..c19a276464e1e 100644
--- a/llvm/lib/Analysis/Loads.cpp
+++ b/llvm/lib/Analysis/Loads.cpp
@@ -313,6 +313,13 @@ bool llvm::isDereferenceableAndAlignedInLoop(LoadInst *LI, Loop *L,
     const auto *Offset = dyn_cast<SCEVConstant>(StartS->getOperand(0));
     const auto *NewBase = dyn_cast<SCEVUnknown>(StartS->getOperand(1));
     if (StartS->getNumOperands() == 2 && Offset && NewBase) {
+      // The following code below assumes the offset is unsigned, but GEP
+      // offsets are treated as signed so we can end up with a signed value
+      // here too. For example, suppose the initial PHI value is (i8 255),
+      // the offset will be treated as (i8 -1) and sign-extended to (i64 -1).
+      if (Offset->getAPInt().isNegative())
+        return false;
+
       // For the moment, restrict ourselves to the case where the offset is a
       // multiple of the requested alignment and the base is aligned.
       // TODO: generalize if a case found which warrants

``````````

</details>


https://github.com/llvm/llvm-project/pull/99490