[llvm-commits] [PATCH] Multidimensional Array Index Delinearization Analysis

[Hal Finkel]

Sebastian,

Thanks for working on this!

A few quick comments:

+namespace {
+struct SCEVGcd : public SCEVVisitor<SCEVGcd, const SCEV *> {

SCEVGcd looks ugly to be, I think that SCEVGCD would be better (actually, I'd replace Gcd with GCD everywhere).

+    if (const SCEVConstant *Cst = dyn_cast<SCEVConstant>(Gcd)) {
+      const SCEV *Res = SE.getConstant(APIntOps::GreatestCommonDivisor(
+          Cst->getValue()->getValue(), Constant->getValue()->getValue()));
+      if (Res != One)
+        return Res;
+    }

Are you sure that Cst and the constant will always have the same number of bits? (what if we've walked through a sign extension, truncation, etc.)

I have the same comment about visitConstant in SCEVDivision.

+  // Removes from Start all multiples of Step and Remainder. The division is
+  // guaranteed to succeed as Step and Remainder have been pattern matched with
+  // SCEVGcd::findGcd().

It seems like there should be an assertion somewhere in here to very this.

+attributes #0 = { nounwind uwtable "less-precise-fpmad"="false" "no-frame-pointer-elim"="false" "no-infs-fp-math"="false" "no-nans-fp-math"="false" "stack-protector-buffer-size"="8" "unsafe-fp-math"="false" "use-soft-float"="false" }
+
+!0 = metadata !{metadata !"int", metadata !1}
+!1 = metadata !{metadata !"omnipotent char", metadata !2}
+!2 = metadata !{metadata !"Simple C/C++ TBAA"}

Remove the unneeded attributes and TBAA metadata from the tests.

 -Hal

----- Original Message -----
> Hi,
> 
> Andrew Trick wrote:
> > 
> > On Oct 10, 2013, at 11:43 AM, Sebastian Pop <spop at codeaurora.org>
> > wrote:
> > > Probably an easier way would be to implement an ad-hoc pattern
> > > matching to
> > > figure out whether Step occurs in each operand of Start, and that
> > > would also
> > > give me a simple way to compute the remainder in case it does not
> > > divide one of
> > > the operands.
> > > 
> > > Let me know what would be the best to implement.
> > 
> > Sure. Sameer pointed out in this thread that the general problem is
> > one of
> > finding a GCD of the start and step. But in this situation there's
> > nothing
> > wrong with pattern matching common cases.
> 
> Thanks Andy for your guidance on this.  Attached is the
> delinearization
> implemented on top of SCEV analysis with pattern matching
> implementation of gcd
> and div.  I have added all testcases from the original patch of Hal
> (also
> attached for reference) all these testcases pass with no regressions
> (with
> respect to Hal's implementation.)
> 
> I have also integrated the delinearization in Preston's dependence
> analysis.  I
> have updated the test-suite with what is now recognized as
> dependences with the
> -da-delinearize flag turned on.  Preston, could you please review
> these changes,
> I am not sure I agree with all these changes to the test-suite, and
> that means
> we will have to adjust either the delinearization or the dependence
> analysis
> pass.
> 
> Thanks for reviewing the two attached patches (Hal's patch is
> provided for
> reference purposes only),
> 
> Sebastian
> --
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> hosted by The Linux Foundation
> 

-- 
Hal Finkel
Assistant Computational Scientist
Leadership Computing Facility
Argonne National Laboratory