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

[Sebastian Pop]

Thanks Hal for the review! It much improved the code:

Hal Finkel wrote:
> +  // 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.

After I added this assert, I had to rework all the GCD and division code for
them to work in sync.  As a result, the changes to the data dependence testsuite
look much better.

> +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).

fixed.

> 
> +    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.

I have not understood what this comment implies and whether there should be an
extra assert or change in the code.  Hal, could you please explain a bit more in
which situation you see this a problem?


> +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.

done.

Attached the revised patch.

Thanks,
Sebastian
-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-delinearization-of-arrays.patch
Type: text/x-diff
Size: 74892 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20131102/0ebbe889/attachment.patch>