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

hfinkel at anl.gov hfinkel at anl.gov
Fri Feb 6 11:24:46 PST 2015


================
Comment at: lib/Transforms/Scalar/StraightLineStrengthReduce.cpp:242
@@ +241,3 @@
+        TTI->isLegalAddressingMode(
+            GEP->getType(), /*BaseGV=*/nullptr, /*BaseOffset=*/0,
+            /*HasBaseReg=*/true,
----------------
Shouldn't you test whether or not B isa GlobalVariable? Seems like, if it is, you should set BaseGV to that, and HasBaseReg = false, and otherwise keep the current set of arguments.

================
Comment at: lib/Transforms/Scalar/StraightLineStrengthReduce.cpp:317
@@ +316,3 @@
+
+void StraightLineStrengthReduce::allocateCandidateAndFindBasisForGEP(
+    GetElementPtrInst *I) {
----------------
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.

http://reviews.llvm.org/D7459

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






More information about the llvm-commits mailing list