[PATCH] [SLSR] handle candidate form &B[i * S]

Jingyue Wu jingyue at google.com
Fri Feb 6 15:25:28 PST 2015


================
Comment at: lib/Transforms/Scalar/StraightLineStrengthReduce.cpp:317
@@ +316,3 @@
+
+void StraightLineStrengthReduce::allocateCandidateAndFindBasisForGEP(
+    GetElementPtrInst *I) {
----------------
hfinkel wrote:
> I don't think this is the right approach, we're inventing ScalarEvolution all over again. SE also already handles the shift case you mention in the TODO comment below.
> 
> If I take your test/Transforms/StraightLineStrengthReduce/X86/no-slsr.ll, for example, and run:
> 
>   opt -analyze -scalar-evolution
> 
> We get:
> 
>   Printing analysis 'Scalar Evolution Analysis' for function 'slsr_gep':
>   Classifying expressions for: @slsr_gep
>     %p0 = getelementptr inbounds i32* %input, i64 0
>     -->  %input
>     %v0 = load i32* %p0
>     -->  %v0
>     %p1 = getelementptr inbounds i32* %input, i64 %s
>     -->  ((4 * %s)<nsw> + %input)<nsw>
>     %v1 = load i32* %p1
>     -->  %v1
>     %s2 = mul nsw i64 %s, 2
>     -->  (2 * %s)
>     %p2 = getelementptr inbounds i32* %input, i64 %s2
>     -->  ((8 * %s) + %input)<nsw>
>     %v2 = load i32* %p2
>     -->  %v2
>     %1 = add i32 %v0, %v1
>     -->  (%v1 + %v0)
>     %2 = add i32 %1, %v2
>     -->  (%v2 + %v1 + %v0)
>   Determining loop execution counts for: @slsr_gep
> 
> And you can see, it nicely decomposes the GEPs into arithmetic expressions for you.
> 
> Andy, please shout if you disagree, and I apologize for not thinking about this before, but I'm inclined to recommend that we rework this pass so that all cases are handled by SE before we get too far. LSR, of course, uses SE, and I think this pass should too for many of the same reasons (even though there is no loop handling here, we need all of the same arithmetic normalization logic, which is fairly significant).
> 
> Also, I think you should consider handling the constant-offset case in GEPs because that will let us get cases where the last index in a structure index. like B[i*J].x, which is fairly common. You just need to make sure such an offset gets passed to isLegalAddressingMode (BaseOffset) above.
Hi Hal, 

Thanks for pointing this out! I original thought SCEV is for loop optimizations and didn't quite think along that line. I'll try to replace BaseExpr or even most of the Candidate class with SCEV. I need to run experiments to see how well this approach works, but in general, I am aligned with you that reusing SCEV is a better approach. 

Jingyue

http://reviews.llvm.org/D7459

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the llvm-commits mailing list