[PATCH] [LoopVectorize]Teach Loop Vectorizer about interleaved memory access
Hao Liu
Hao.Liu at arm.com
Tue May 19 03:45:41 PDT 2015
I've updated a new patch according to the comments from Renato and Adam. This patch adds a new class InterleavedAccessInfo to handle the analysis about interleaved accesses.
Review please.
Thanks,
-Hao
================
Comment at: include/llvm/Analysis/LoopAccessAnalysis.h:401
@@ +400,3 @@
+
+ delete this;
+ }
----------------
rengolin wrote:
> Ouch, no!
>
> If you do this, than a perfectly valid sequence like this:
>
> group.eraseFromMap(Map);
> group.getDelta(); // there's no way to know that this won't work
>
> will segfault.
>
> A quick fix would be to leave the deletion to whomever is calling the function, so it's clear on the lifetime of the object:
>
> group.eraseFromMap(Map);
> group.getDelta();
> delete group;
> group.getReverse(); // this is obviously wrong
>
> But again, it seems to me that such logic is spread out too much.
>
> So a better fix would be to coalesce all of it into a class in itself. One that contains the maps, the groups, and the control over the lifetime of all its groups in a consistent way, without requiring the caller to know about it.
>
> I'm ok with the temporary solution, as long as you add a FIXME to that effect.
That's reasonable.
I've added a new class InterleavedAccessInfo to manage this.
================
Comment at: include/llvm/Analysis/LoopAccessAnalysis.h:670-671
@@ -488,1 +669,4 @@
+
+ /// Contains the relationships between the members and the interleaved access.
+ DenseMap<Instruction *, InterleaveGroup *> InterleaveGroupMap;
};
----------------
anemet wrote:
> I've only started looking at this patch but I have a quick initial, more fundamental comment.
>
> You are modifying an analysis pass (LoopAccessAnalysis). Please add support for printing the new information gathered by the analysis. Please also add tests under test/Analysis/LoopAccessAnalysis to verify the new information.
>
> You may even want separate out the patch for the analysis part (LAA + tests) from the transformation parts (LV + tests + etc).
Hi Adam,
I've undated the patch to support printing the new information. I added more test cases as well.
I didn't separate patch as I think there are already a lot of history comments on both parts. Also I think the bound between analysis part and transformation part is clear.
Anyway, I can still separate this patch if you insist.
http://reviews.llvm.org/D9368
EMAIL PREFERENCES
http://reviews.llvm.org/settings/panel/emailpreferences/
More information about the llvm-commits
mailing list