[PATCH] D98967: [Analysis]Add getPointersDiff function to improve compile time.
Alexey Bataev via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri Mar 19 12:40:41 PDT 2021
ABataev added inline comments.
================
Comment at: llvm/lib/Analysis/LoopAccessAnalysis.cpp:1129-1134
+ unsigned ASA = PtrA->getType()->getPointerAddressSpace();
+ unsigned ASB = PtrB->getType()->getPointerAddressSpace();
+
+ // Check that the address spaces match and that the pointers are valid.
+ if (!PtrA || !PtrB || (ASA != ASB) || PtrA->getType() != PtrB->getType())
+ return None;
----------------
lebedev.ri wrote:
>
Ok, will do
================
Comment at: llvm/lib/Analysis/LoopAccessAnalysis.cpp:1158-1159
+
+ OffsetA = OffsetA.sextOrTrunc(IdxWidth);
+ OffsetB = OffsetB.sextOrTrunc(IdxWidth);
+
----------------
lebedev.ri wrote:
>
It is used for `Size`
================
Comment at: llvm/lib/Analysis/LoopAccessAnalysis.cpp:1161-1165
+ // OffsetDelta = OffsetB - OffsetA;
+ const SCEV *OffsetSCEVA = SE.getConstant(OffsetA);
+ const SCEV *OffsetSCEVB = SE.getConstant(OffsetB);
+ const SCEV *OffsetDeltaSCEV = SE.getMinusSCEV(OffsetSCEVB, OffsetSCEVA);
+ const APInt &OffsetDelta = cast<SCEVConstant>(OffsetDeltaSCEV)->getAPInt();
----------------
lebedev.ri wrote:
> Why not just do this in APInt directly?
Will try
================
Comment at: llvm/lib/Analysis/LoopAccessAnalysis.cpp:1167-1172
+ // Check if they are based on the same pointer. That makes the offsets
+ // sufficient.
+ int Val = OffsetDelta.getSExtValue() / Size.getSExtValue();
+ if (Val * Size == OffsetDelta)
+ return Val;
+ return None;
----------------
lebedev.ri wrote:
> But this checks that `OffsetDelta` is a multiple of original pointed-to type,
> not what the comment says?
>
> I suppose this overly restrictive.
1. Yes, the comment is wrong, will fix it.
2. Unfortunately not. Caused by a real example.
================
Comment at: llvm/lib/Analysis/LoopAccessAnalysis.cpp:1180-1185
+ if (Diff) {
+ int Val = Diff->getAPInt().getSExtValue() / Size.getSExtValue();
+ if (Val * Size == Diff->getAPInt())
+ return Val;
+ }
+ return None;
----------------
lebedev.ri wrote:
> Would it be better to add the non-constant case as an `else` to `if (PtrA1 == PtrB1) {`,
> and deduplicate this code?
I'll try
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D98967/new/
https://reviews.llvm.org/D98967
More information about the llvm-commits
mailing list